{"thread":{"id":"61542","subject":"[PATCH] alias: pass --help through to shell alias","startedAt":"2024-05-24T07:07:09Z","lastAt":"2024-05-25T01:36:17Z","messageCount":3,"participants":["Ian Wienand","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"495515","messageId":"20240524070623.1344636-2-iwienand@redhat.com","threadId":"61542","inReplyTo":null,"subject":"[PATCH] alias: pass --help through to shell alias","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-24T07:04:14Z","receivedAt":"2024-05-24T07:07:09Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"In trying to make some aliases more consistent in an internal tool, I\nimplemented both -h/--help in the underlying command for a shell\nalias, and was a bit surprised when \"git <alias> -h\" worked but \"git\n<alias> --help\" didn't.\n\nIn the \"-h\" case in git.c:handle_alias() we have a little check for\n\"-h\" which pre-prints the \"<alias> is aliased to...\" line and then\ncontinues to run the alias.\n\nHowever in the \"--help\" case we fall into the logic that turns \"git\ncmd --help\" into \"git help cmd\"\n\nhelp.c contains a check to just print the alias mapping if called as\n\"git help <alias>\", but if called as \"git <alias> --help\", will\nredirect to the help of the aliased-command.  Since a shell alias does\nnot have a 1:1 mapping with an internal command, the current check\nprints the alias mapping and simply exits in that case (causing my\nalias script \"--help\" command to never trigger).\n\nI would propose that \"--help\" to a shell alias is passed through to\nthe underlying command.  This way you can write aliases that act more\nlike the other git commands.\n\nTo do this, we make \"--help\" work the same as \"-h\" for shell aliases.\nIn git.c where we check for the \"--help\" argument, for shell aliases\nwe print the aliased command as \"-h\" does, but then shortcut the\nrewriting to \"git help\", so the aliased command will run and not be\nsent to \"git help\".\n\nSince \"git <alias> --help\" will not be re-written for shell aliases,\n\"git help\" can now assume that it is being asked to help on a\ngit-command alias.  Thus we can remove the \"is this a rewritten shell\nalias\" check.\n\nA test-case is added to ensure \"--help\" is passed through to the\nunderlying command of a shell alias.\n\nSigned-off-by: Ian Wienand <iwienand@redhat.com>\n---\n builtin/help.c   |  7 +++----\n git.c            | 15 +++++++++++++++\n t/t0014-alias.sh |  8 ++++++++\n 3 files changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 222f994f86..5e9d5edbb2 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -547,11 +547,10 @@ static const char *check_git_cmd(const char* cmd)\n \t\t * handle_builtin() in git.c rewrites \"git cmd --help\"\n \t\t * to \"git help --exclude-guides cmd\", so we can use\n \t\t * exclude_guides to distinguish \"git cmd --help\" from\n-\t\t * \"git help cmd\". In the latter case, or if cmd is an\n-\t\t * alias for a shell command, just print the alias\n-\t\t * definition.\n+\t\t * \"git help cmd\". In the latter case, just print the\n+\t\t * alias definition.\n \t\t */\n-\t\tif (!exclude_guides || alias[0] == '!') {\n+\t\tif (!exclude_guides) {\n \t\t\tprintf_ln(_(\"'%s' is aliased to '%s'\"), cmd, alias);\n \t\t\tfree(alias);\n \t\t\texit(0);\ndiff --git a/git.c b/git.c\nindex 3d8e48cf55..4c93296550 100644\n--- a/git.c\n+++ b/git.c\n@@ -691,12 +691,27 @@ static void strip_extension(const char **argv)\n static void handle_builtin(int argc, const char **argv)\n {\n \tstruct strvec args = STRVEC_INIT;\n+\tchar *alias;\n \tconst char *cmd;\n \tstruct cmd_struct *builtin;\n \n \tstrip_extension(argv);\n \tcmd = argv[0];\n \n+\t/*\n+\t * If this is a shell alias with --help, print it's alias\n+\t * mapping to be consistent with -h and pass it through\n+\t */\n+\talias = alias_lookup(cmd);\n+\tif (alias && alias[0] == '!') {\n+\t\tif (argc > 1 && !strcmp(argv[1], \"--help\")) {\n+\t\t\tfprintf_ln(stderr, _(\"'%s' is aliased to '%s'\"), cmd, alias);\n+\t\t\tfree(alias);\n+\t\t\treturn;\n+\t\t}\n+\t\tfree(alias);\n+\t}\n+\n \t/* Turn \"git cmd --help\" into \"git help --exclude-guides cmd\" */\n \tif (argc > 1 && !strcmp(argv[1], \"--help\")) {\n \t\tint i;\ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 8d3d9144c0..7b1d559420 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -44,4 +44,12 @@ test_expect_success 'run-command formats empty args properly' '\n     test_cmp expect actual\n '\n \n+test_expect_success '--help is passed to execed alias' '\n+    echo \"echo \\\"\\$@\\\"\" > exec-alias.sh &&\n+    chmod +x exec-alias.sh &&\n+    git config alias.exec-alias \"!\\\"$(pwd)/exec-alias.sh\\\"\" &&\n+    GIT_TRACE=1 git exec-alias --help &> output &&\n+    test_i18ngrep -- \"--help\" output\n+'\n+\n test_done\n-- \n2.45.1\n\n"},{"id":"495610","messageId":"xmqq8qzyk9u8.fsf@gitster.g","threadId":"61542","inReplyTo":"20240524070623.1344636-2-iwienand@redhat.com","subject":"Re: [PATCH] alias: pass --help through to shell alias","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-25T00:34:55Z","receivedAt":"2024-05-25T00:35:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand <iwienand@redhat.com> writes:\n\n> In trying to make some aliases more consistent in an internal tool, I\n> implemented both -h/--help in the underlying command for a shell\n> alias, and was a bit surprised when \"git <alias> -h\" worked but \"git\n> <alias> --help\" didn't.\n\nYeah, with\n\n    alias.lg=log --oneline\n    alias.lgm=!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -\n\n\"git lg -h\" reports the alias, \"git lg --help\" does the same as \"git\nlog --help\", \"git lgm -h\" reports the alias, but \"git lgm --help\"\nrefrains from doing \n\n    sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' - --help\n\nand instead does the same as \"git lgm -h\" to report the alias.\n\nThis is a safe behaviour because the underlying command may not be\nprepared to see \"--help\" and ignore it silently in the best case,\ne.g.\n\n    sh -c 'false \"$@\" || :' - --help\n\nthat would confuse users by being totally silent, or \n\n    sh -c 'awk \"$@\" || :' - --help\n\nthat gives \"awk: not an option: --help\" (which is less useful than\nthe report of alias), or even worse yet, if the underlying command\ndoes not understand \"--help\" and considers it something different,\nwho knows what havoc it would wreak.\n\nFor the above reason ...\n\n> I would propose that \"--help\" to a shell alias is passed through to\n> the underlying command.  This way you can write aliases that act more\n> like the other git commands.\n\n... this is a dangerous thing to do unconditionally.\n\nI wonder if we can come up with a notation to annotate aliases that\ndo support the \"--help\" option that wouldn't have been used by\nmistake for existing aliases?\n"},{"id":"495617","messageId":"ZlFAiMAOaBK1Qrrw@fedora19.localdomain","threadId":"61542","inReplyTo":"xmqq8qzyk9u8.fsf@gitster.g","subject":"Re: [PATCH] alias: pass --help through to shell alias","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T01:36:08Z","receivedAt":"2024-05-25T01:36:17Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Fri, May 24, 2024 at 05:34:55PM -0700, Junio C Hamano wrote:\n> Ian Wienand <iwienand@redhat.com> writes:\n> This is a safe behaviour because the underlying command may not be\n> prepared to see \"--help\" and ignore it silently in the best case,\n> e.g.\n> \n>     sh -c 'false \"$@\" || :' - --help\n> \n> that would confuse users by being totally silent, or \n> \n>     sh -c 'awk \"$@\" || :' - --help\n> \n> that gives \"awk: not an option: --help\" (which is less useful than\n> the report of alias),\n\nThe proposed patch does still print the alias expansion before trying\nto pass the \"--help\" onto the command.  That was intention to be the\nsame as \"-h\" behaviour.  But I take your point the error message\ndoesn't help.\n\n> or even worse yet, if the underlying command\n> does not understand \"--help\" and considers it something different,\n> who knows what havoc it would wreak.\n\nI guess this means that someone was relying on git to filter out\n--help such that it would never get to their command.  To me this\ncould probably be considered \"undefined behaviour\" that is probably\nlow risk to change, but I understand the aversion to it.\n\n> For the above reason ...\n> \n> > I would propose that \"--help\" to a shell alias is passed through to\n> > the underlying command.  This way you can write aliases that act more\n> > like the other git commands.\n> \n> ... this is a dangerous thing to do unconditionally.\n> \n> I wonder if we can come up with a notation to annotate aliases that\n> do support the \"--help\" option that wouldn't have been used by\n> mistake for existing aliases?\n\nWe could do something like make \"!!\" as an alias prefix also passes\nthis through.  It seems too niche to bother with though, TBH.\n\nI could propose documenting the behaviour as-is, although that\nprobably moves it from undefined behaviour to \"this will never\nchange\".  I'll take advice on what you think :)\n\n-i\n\n"}]}