Implement switch -h, --help to CLI

[nappex]

Fix #205

[Glutexo]

Tests? 🤔

[nappex]

Tests? 🤔

thanks for reminding 😮‍💨

[Glutexo]

I think I found an error. If a value is provided for the -h switch, MatchError is raised.

$ ./onigumo -h value
** (MatchError) no match of right hand side value: :error
    (onigumo 0.1.0) lib/cli.ex:16: Onigumo.CLI.main/1
    (elixir 1.17.2) lib/kernel/cli.ex:136: anonymous fn/3 in Kernel.CLI.exec_fun/2

[Glutexo]

Update: the error I found is not a problem of this pull request. Its happens when an invalid component (a positional argument) is passed. It is a known issue #179.

[Glutexo]

Unfortunately, a combination of -h/--help and a positional argument doesn’t work as expected. See my inline comment.

[Glutexo]

Capitalized, this sentence would be more consistent.

Suggested change

	-h, --help\t\tprint this help
	-h, --help\t\tPrint this help

[Glutexo]

There is one more user story of providing both a positional argument and the help switch. If ran in the ./onigumo downloader -h order, the -h switch is ignored.

This case is not tested and because it doesn’t work correctly, the test would fail. The expected result is a help message to be printed because of an invalid argument combination. The actual result is that the downloader is run.

[nappex]

that should be already tested but it is strange that github does not mark as as outdated. I suppose it because the line of code you mark is not changed but in text is mentioned case of test which is not added in your changes.

[Glutexo]

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]

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]

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]

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

[nappex]

--help must be used as only one cli argument to run correctly, we do not want behaviour as "rule them all" by help switch as proposed in PR #244.
If someone use --help with other argument or switch, then it could evoke to user it should output help for certain argument resp. switch. We do not want to implement this feature currently, maybe later. So --help should be used only for general help message for whole Onigumo not for its individual commands.