[PATCH] alias: pass --help through to shell alias

[PATCH] alias: pass --help through to shell alias

[Ian Wienand]

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(-)

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
-- 
2.45.1

Re: [PATCH] alias: pass --help through to shell alias

[Junio C Hamano]

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?

Re: [PATCH] alias: pass --help through to shell alias

[Ian Wienand]

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