On Mon, Nov 09, 2020 at 01:03:47AM +0100, Pavel Sanda wrote:
> > > > @@ -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.
So I looked at the code for the command line run at this place looks good
enough.
I have another concerns though:
1) slotOK contains error() call which seems to be missed now. If you don't want
it for
commandline run it should be conditioned on cmd_mode.
2) It seems that if (!cmd_mode) you will call finished(false) twice thus wrongly
reverting the toggles from FuncRequest(LFUN_CHANGES_OUTPUT) in finished(),
right?
Small glitches:
2) Please add explaining comment for the cmd_mode parameter to the header.
3) As said in previous mail I would set timer either to infinity (or to
something
realistically short if you insist on some timing wall).
Pavel
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
