[Patch] Test suite for compare function

Sam Crawley sam at crawley.nz
Wed Nov 11 08:10:09 UTC 2020


On Wed, 11 Nov 2020, at 00:31, Pavel Sanda wrote:
> So I looked at the code for the command line run at this place looks 
> good enough.

Thanks for having a look.

> 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.

Yes, fair point. I'll add the error checking back in when it's run via the command line.

> 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?

I'm not sure I understand. When cmd_mode == true (which is always the case when run() is invoked via the LFUN), then finished will always be called after the wait(), and only once. If cmd_mode == false (which is always the case when run() is otherwise invoked) then finished() is connected to the finished signal from the Compare worker thread. So I'm not seeing the case where finished is called twice, although I may well have missed something.

> 
> 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).

Thanks, I'll fix those, and update the patchset soon (with some of the other suggestions in this thread as well).

Sam.


More information about the lyx-devel mailing list