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.
>
>
> > src/frontends/qt/GuiCompare.cpp | 28 +++++++++++++++++++---------
> > src/frontends/qt/GuiCompare.h | 2 +-
> > 2 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/frontends/qt/GuiCompare.cpp
> > b/src/frontends/qt/GuiCompare.cpp
> > index d1da5b337f..380244a5c3 100644
> > --- a/src/frontends/qt/GuiCompare.cpp
> > +++ b/src/frontends/qt/GuiCompare.cpp
> > @@ -42,7 +42,7 @@ namespace frontend {
> >
> > GuiCompare::GuiCompare(GuiView & lv)
> > : GuiDialog(lv, "compare", qt_("Compare LyX files")),
> > - compare_(0), dest_buffer_(0), old_buffer_(0), new_buffer_(0)
> > + compare_(0), dest_buffer_(0), old_buffer_(0), new_buffer_(0)
>
> If possible try to separate patches with whitespace only changes and
> real code changes.
Yes, I'll fix that.
>
> > @@ -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.
>
> 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.
Thanks,
Sam.
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
