On Mon, Nov 09, 2020 at 09:19:42AM +1300, Sam Crawley wrote:
> On Sun, 8 Nov 2020, at 23:32, Pavel Sanda wrote:
>
> > Git commit messages tend to have the following structure: first summary
> > line,
> > empty line and then the details. This helps with log summaries.
>
> That is the format I used, unless I'm missing something. The 'subject' line
> in a git patch file is the first line of the commit message.
My oversight, it's indeed inside subject line.
> > > @@ -343,7 +348,12 @@ bool GuiCompare::initialiseParams(std::string const
> > > &par)
> > > if (cmd.getArg(0) == "run") {
> > > oldFileCB->setEditText(toqstr(cmd.getArg(1)));
> > > newFileCB->setEditText(toqstr(cmd.getArg(2)));
> > > - slotOK();
> > > + enableControls(false);
> > > + run(true);
> > > +
> > > + compare_->wait(1000000);
> >
> > On a first sight this looks fishy. We unconditionally launch run
> > on the place we previously did not without dependency on cmd_mode.
> > Do I miss something?
>
> Previously run() was called through slotOK(). The problem is, when invoking
> compare from the command line, the next command in the sequence was getting
> executed before the compare was finished because the compare code was getting
> executed in a thread (and so buffer-write-as couldn't find a buffer to write,
> as it had not yet been created).
>
> Just adding the 'wait()' call was not enough, because the 'finished' signal
> still happened asynchronously, so I also added the parameter to run() so that
> the finished signal is not connected to (and then called the finished()
> method manually afterwards). There may be a cleaner way to do this, I'm happy
> to hear suggestions. But the current approach at least didn't seem terrible
> to me. It probably could use a comment to explain this, though, which I'll
> add.
Ok, I will dig into the code later. Launching run inside initialiseParams looks
strange.
> > Secondly instead of waiting for arbitrary time, can't we just wait
> > until it finishes?
>
> The parameter to wait is just a timeout. It will block until the compare
> thread is complete, or the timeout is reached.
I understand that, but unless we have reasonable timeout (not two weeks) the
code with unconditional wait looks cleaner to me.
Pavel
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
