Handle all Qt command line options (--help-all)
ClosedPublic

Authored by yuyichao on Apr 9 2020, 2:17 PM.

Details

Summary

This was broken in c5897d0eabf515a82a62429a726819cb60e255ae suggesting
that it might be needed to

fix --help, --version and option errors not being reported on the
terminal if another instance of KAlarm is already running.

Testing suggests that this is not the case, especially since
the printing to the terminal was handled directly by process
in which case it'll never return. I don't really see how the
printing could be stopped unless stdout or stderr is overriden,
which doesn't seem to be the case.

This is the same as how KDE builtin options are currently handled.

Also remove now unused EXIT command.

Diff Detail

Repository
R205 KAlarm
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
yuyichao created this revision.Apr 9 2020, 2:17 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptApr 9 2020, 2:17 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
yuyichao requested review of this revision.Apr 9 2020, 2:17 PM

I can't find any difference between KAlarm's behaviour before and after applying the patch. The --help-all option isn't recognised in either case. (I'm building with Qt 5.12: perhaps --help-all is now recognised by a later version of Qt, like it was in Qt4 days?) Other KDE applications on my system also don't recognise --help-all.

That's possible. This appears to be added (back?) with https://code.qt.io/cgit/qt/qtbase.git/commit/?id=341c8b9cd08 last May which is newer than the 5.12 branch point it seems. My kalarm --help (as well as most other KDE/Qt programs) shows (a variant of),

Usage: kalarm [options] [message]
KAlarm

Options:
  -h, --help                         Displays help on commandline options.
  --help-all                         Displays help including Qt specific
                                     options.
  -v, --version                      Displays version information.

which is why I was surprised that kalarm --help-all throws no error and just starts normally as if I've passed in nothing. I assume your --help does not include a --help-all option. Searching the doc this appears to be added in 5.14. 5.13 doc doesn't have it. (Qt doesn't seem to have a stable link for the latest specific version so https://doc.qt.io/qt-5/qcommandlineparser.html#addHelpOption should show the 5.14 doc before 5.15 is released)

Thinking about it, I think this is an argument to use process instead of handling it manually. Qt clearly make changes to what builtin options there are....

djarvie accepted this revision.Apr 9 2020, 3:50 PM

Thanks for the information about --help-all.

Your patch allows for extra built-in options to be processed without the need for the KAlarm code to know about them. For that reason, it's a good idea. Thanks.

This revision is now accepted and ready to land.Apr 9 2020, 3:50 PM
This revision was automatically updated to reflect the committed changes.