On Wed, Mar 30 2022, Garrit Franke wrote:
> If "-v" or "-h" is passed as a flag, they will be interpreted as
> a "version" or "help" command respectively.
They won't, try e.g. "./git -h" before/after this change. On a second
reading I see that there's an implied "change the behavior to ..."
there.
But it would really help if the commit message discussed what we did
before, what we do now, and why the change is safe/OK.
I *think* we just emit the message about an unknown option before, and
this makes it synonymous with --verbose/verbose and --help/(but not
"help"), but saying so would be useful :)
> Since this is my first patch for this project, I'm very happy to take
> your feedback!
Welcome to git development!
> Documentation/git.txt | 4 +++-
> git.c | 15 ++++++++++++---
> po/bg.po | 4 ++--
> po/ca.po | 4 ++--
> po/de.po | 4 ++--
> po/el.po | 4 ++--
> po/es.po | 4 ++--
> po/fr.po | 4 ++--
> po/git.pot | 2 +-
> po/id.po | 4 ++--
> po/it.po | 4 ++--
> po/ko.po | 4 ++--
> po/pl.po | 4 ++--
> po/pt_PT.po | 2 +-
> po/ru.po | 4 ++--
> po/sv.po | 4 ++--
> po/tr.po | 4 ++--
> po/vi.po | 4 ++--
> po/zh_CN.po | 4 ++--
> po/zh_TW.po | 4 ++--
This *.po stuff is a well-meaning change, but please leave this out of
the patch.
The translation files are updated by a process that the translation
teams(s) use, see po/README.md. I think changing this from under them is
probably more disruptive than not.
> - if (!strcmp(cmd, "--help") || !strcmp(cmd, "--version"))
> + if (!strcmp(cmd, "-h") ||
> + !strcmp(cmd, "--help") ||
> + !strcmp(cmd, "-v") ||
> + !strcmp(cmd, "--version"))
nit: minimize the diff here perhaps by keeping the existing line, and
doing -h or -v on a new line? Your indentation here is also off.
> @@ -893,7 +896,13 @@ int cmd_main(int argc, const char **argv)
> handle_options(&argv, &argc, NULL);
> if (argc > 0) {
> /* translate --help and --version into commands */
> - skip_prefix(argv[0], "--", &argv[0]);
> + if (!strcmp("-v", argv[0])) {
> + argv[0] = "version";
> + } else if (!strcmp("-h", argv[0])) {
> + argv[0] = "help";
> + } else {
> + skip_prefix(argv[0], "--", &argv[0]);
> + }
Don't use {} braces here, see CodingGudielines.
FWIW I have an existing branch (unsubmitted) at
https://github.com/avar/git/tree/avar/parse-options-h-3 where I added
some tests for "git cmd -h" behavior, which seems to pass with this
change (not unexpected, as this is for the top-level command).
