| Message ID | 20240524070623.1344636-2-iwienand@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | alias: pass --help through to shell alias | expand |
Ian Wienand <iwienand@redhat.com> writes: > In trying to make some aliases more consistent in an internal tool, I > implemented both -h/--help in the underlying command for a shell > alias, and was a bit surprised when "git <alias> -h" worked but "git > <alias> --help" didn't. Yeah, with alias.lg=log --oneline alias.lgm=!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log "$@" || :' - "git lg -h" reports the alias, "git lg --help" does the same as "git log --help", "git lgm -h" reports the alias, but "git lgm --help" refrains from doing sh -c 'GIT_NOTES_REF=refs/notes/amlog git log "$@" || :' - --help and instead does the same as "git lgm -h" to report the alias. This is a safe behaviour because the underlying command may not be prepared to see "--help" and ignore it silently in the best case, e.g. sh -c 'false "$@" || :' - --help that would confuse users by being totally silent, or sh -c 'awk "$@" || :' - --help that gives "awk: not an option: --help" (which is less useful than the report of alias), or even worse yet, if the underlying command does not understand "--help" and considers it something different, who knows what havoc it would wreak. For the above reason ... > I would propose that "--help" to a shell alias is passed through to > the underlying command. This way you can write aliases that act more > like the other git commands. ... this is a dangerous thing to do unconditionally. I wonder if we can come up with a notation to annotate aliases that do support the "--help" option that wouldn't have been used by mistake for existing aliases?
On Fri, May 24, 2024 at 05:34:55PM -0700, Junio C Hamano wrote: > Ian Wienand <iwienand@redhat.com> writes: > This is a safe behaviour because the underlying command may not be > prepared to see "--help" and ignore it silently in the best case, > e.g. > > sh -c 'false "$@" || :' - --help > > that would confuse users by being totally silent, or > > sh -c 'awk "$@" || :' - --help > > that gives "awk: not an option: --help" (which is less useful than > the report of alias), The proposed patch does still print the alias expansion before trying to pass the "--help" onto the command. That was intention to be the same as "-h" behaviour. But I take your point the error message doesn't help. > or even worse yet, if the underlying command > does not understand "--help" and considers it something different, > who knows what havoc it would wreak. I guess this means that someone was relying on git to filter out --help such that it would never get to their command. To me this could probably be considered "undefined behaviour" that is probably low risk to change, but I understand the aversion to it. > For the above reason ... > > > I would propose that "--help" to a shell alias is passed through to > > the underlying command. This way you can write aliases that act more > > like the other git commands. > > ... this is a dangerous thing to do unconditionally. > > I wonder if we can come up with a notation to annotate aliases that > do support the "--help" option that wouldn't have been used by > mistake for existing aliases? We could do something like make "!!" as an alias prefix also passes this through. It seems too niche to bother with though, TBH. I could propose documenting the behaviour as-is, although that probably moves it from undefined behaviour to "this will never change". I'll take advice on what you think :) -i
diff --git a/builtin/help.c b/builtin/help.c index 222f994f86..5e9d5edbb2 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -547,11 +547,10 @@ static const char *check_git_cmd(const char* cmd) * handle_builtin() in git.c rewrites "git cmd --help" * to "git help --exclude-guides cmd", so we can use * exclude_guides to distinguish "git cmd --help" from - * "git help cmd". In the latter case, or if cmd is an - * alias for a shell command, just print the alias - * definition. + * "git help cmd". In the latter case, just print the + * alias definition. */ - if (!exclude_guides || alias[0] == '!') { + if (!exclude_guides) { printf_ln(_("'%s' is aliased to '%s'"), cmd, alias); free(alias); exit(0); diff --git a/git.c b/git.c index 3d8e48cf55..4c93296550 100644 --- a/git.c +++ b/git.c @@ -691,12 +691,27 @@ static void strip_extension(const char **argv) static void handle_builtin(int argc, const char **argv) { struct strvec args = STRVEC_INIT; + char *alias; const char *cmd; struct cmd_struct *builtin; strip_extension(argv); cmd = argv[0]; + /* + * If this is a shell alias with --help, print it's alias + * mapping to be consistent with -h and pass it through + */ + alias = alias_lookup(cmd); + if (alias && alias[0] == '!') { + if (argc > 1 && !strcmp(argv[1], "--help")) { + fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias); + free(alias); + return; + } + free(alias); + } + /* Turn "git cmd --help" into "git help --exclude-guides cmd" */ if (argc > 1 && !strcmp(argv[1], "--help")) { int i; diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh index 8d3d9144c0..7b1d559420 100755 --- a/t/t0014-alias.sh +++ b/t/t0014-alias.sh @@ -44,4 +44,12 @@ test_expect_success 'run-command formats empty args properly' ' test_cmp expect actual ' +test_expect_success '--help is passed to execed alias' ' + echo "echo \"\$@\"" > exec-alias.sh && + chmod +x exec-alias.sh && + git config alias.exec-alias "!\"$(pwd)/exec-alias.sh\"" && + GIT_TRACE=1 git exec-alias --help &> output && + test_i18ngrep -- "--help" output +' + test_done
In trying to make some aliases more consistent in an internal tool, I implemented both -h/--help in the underlying command for a shell alias, and was a bit surprised when "git <alias> -h" worked but "git <alias> --help" didn't. In the "-h" case in git.c:handle_alias() we have a little check for "-h" which pre-prints the "<alias> is aliased to..." line and then continues to run the alias. However in the "--help" case we fall into the logic that turns "git cmd --help" into "git help cmd" help.c contains a check to just print the alias mapping if called as "git help <alias>", but if called as "git <alias> --help", will redirect to the help of the aliased-command. Since a shell alias does not have a 1:1 mapping with an internal command, the current check prints the alias mapping and simply exits in that case (causing my alias script "--help" command to never trigger). I would propose that "--help" to a shell alias is passed through to the underlying command. This way you can write aliases that act more like the other git commands. To do this, we make "--help" work the same as "-h" for shell aliases. In git.c where we check for the "--help" argument, for shell aliases we print the aliased command as "-h" does, but then shortcut the rewriting to "git help", so the aliased command will run and not be sent to "git help". Since "git <alias> --help" will not be re-written for shell aliases, "git help" can now assume that it is being asked to help on a git-command alias. Thus we can remove the "is this a rewritten shell alias" check. A test-case is added to ensure "--help" is passed through to the underlying command of a shell alias. Signed-off-by: Ian Wienand <iwienand@redhat.com> --- builtin/help.c | 7 +++---- git.c | 15 +++++++++++++++ t/t0014-alias.sh | 8 ++++++++ 3 files changed, 26 insertions(+), 4 deletions(-)