Implement switch -h, --help to CLI

nappex commented 1 month ago

Fix #205

nappex commented 1 month ago

Tests? 🤔

thanks for reminding 😮‍💨

Glutexo commented 3 weeks ago

My observation of onigumo downloader -h not printing the help message is not a matter of this pull request. It is a bug in the code preventing switches from being parsed if a component is provided. onigumo downloader --anything would behave identically.

The question is whether to add a test case for the combination of the help switch and the component. Because of the existing bug, the test would fail and essentially make this pull request blocked. We can keep this test for the fix and by that let this pull request go through.

Glutexo commented 3 weeks ago

In my previous comment, I was wrong! 😮 It’s only the -h/--help switch that is ignored. An invalid switch like -x or --invalid still causes a help message to be printed.

$ ./onigumo downloader -h

$ ./onigumo downloader -x
Usage: onigumo [OPTION]... [COMPONENT]

Simple program that retrieves HTTP web content as structured data.

COMPONENT   Onigumo component to run, available: downloader

OPTIONS:
-h, --help      print this help
-C, --working-dir <dir> Change working dir to <dir> before running

Glutexo commented 2 weeks ago

To make it clearer:

On master, onigumo downloader -h prints a help message, which is correct: it’s an invalid argument combination.
On implement-help-option, the Downloader is run, which is incorrect: the -h option is ignored.

Glutexo commented 2 weeks ago

I found the reason, why the behavior changed:

On master, neither the --help switch nor the -h alias were defined in strict or aliases respectively. Because of that, --help/-h ended up in the invalid section of the result, not matching the empty list in {parsed_switches, [component], []} and proceeding to _ printing a help message.
On implement-help-option, however, the help switches are defined and because of that :help is not put in invalid. [] of invalid switches is then properly matched by {parsed_switches, [component], []} and the help switch ends up unparsed in parsed_switches.

This pull request then does introduce the bug in the sense that in didn’t exist because there was no help option. At the same time, it only reveals an existing issues of unscalable pattern matching of the OptionParser.parse/2 output.

This can be still resolved tactically by using my code suggestion still being purely based on pattern-matching, probably in a separate pull request unblocking this one. The strategic solution is however to replace the pattern matching by something scalable.

Glutexo / onigumo

Implement switch -h, --help to CLI #238