{"thread":{"id":"54609","subject":"[PATCH v2 0/3] completion: bash: support for recursive/nested aliases","startedAt":"2020-11-10T00:53:36Z","lastAt":"2020-11-10T02:09:50Z","messageCount":6,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"409444","messageId":"20201110005331.532678-1-felipe.contreras@gmail.com","threadId":"54609","inReplyTo":null,"subject":"[PATCH v2 0/3] completion: bash: support for recursive/nested aliases","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-10T00:53:28Z","receivedAt":"2020-11-10T00:53:36Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hello,\n\nAs Philippe reported, aliases of aliases are not completed correctly in the\nBash completion. This patch series attempts to fix that problem.\n\nSince v1:\n\n * Use a loop instead of recursive calls\n * Add a check to detect alias loops\n\n\nFelipe Contreras (3):\n  completion: bash: support recursive aliases\n  completion: bash: check for alias loop\n  completion: bash: simplify __git_aliased_command\n\n contrib/completion/git-completion.bash | 53 +++++++++++++++++---------\n t/t9902-completion.sh                  | 19 +++++++++\n 2 files changed, 54 insertions(+), 18 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"409445","messageId":"20201110005331.532678-2-felipe.contreras@gmail.com","threadId":"54609","inReplyTo":"20201110005331.532678-1-felipe.contreras@gmail.com","subject":"[PATCH v2 1/3] completion: bash: support recursive aliases","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-10T00:53:29Z","receivedAt":"2020-11-10T00:53:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It is possible to have a recursive aliases like:\n\n  l = log --oneline\n  lg = l --graph\n\nSo the completion should detect such aliases as well.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 48 ++++++++++++++++----------\n t/t9902-completion.sh                  | 19 ++++++++++\n 2 files changed, 48 insertions(+), 19 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 36f5a91c7a..59ced25641 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1121,26 +1121,36 @@ __git_pretty_aliases ()\n # __git_aliased_command requires 1 argument\n __git_aliased_command ()\n {\n-\tlocal word cmdline=$(__git config --get \"alias.$1\")\n-\tfor word in $cmdline; do\n-\t\tcase \"$word\" in\n-\t\t\\!gitk|gitk)\n-\t\t\techo \"gitk\"\n-\t\t\treturn\n-\t\t\t;;\n-\t\t\\!*)\t: shell command alias ;;\n-\t\t-*)\t: option ;;\n-\t\t*=*)\t: setting env ;;\n-\t\tgit)\t: git itself ;;\n-\t\t\\(\\))   : skip parens of shell function definition ;;\n-\t\t{)\t: skip start of shell helper function ;;\n-\t\t:)\t: skip null command ;;\n-\t\t\\'*)\t: skip opening quote after sh -c ;;\n-\t\t*)\n-\t\t\techo \"$word\"\n-\t\t\treturn\n-\t\tesac\n+\tlocal cur=$1 last word cmdline\n+\n+\twhile [[ \"$cur\" != \"$last\" ]]; do\n+\t\tcmdline=$(__git config --get \"alias.$cur\")\n+\t\tlast=$cur\n+\n+\t\tfor word in $cmdline; do\n+\t\t\tcase \"$word\" in\n+\t\t\t\\!gitk|gitk)\n+\t\t\t\tcur=\"gitk\"\n+\t\t\t\tbreak\n+\t\t\t\t;;\n+\t\t\t\\!*)\t: shell command alias ;;\n+\t\t\t-*)\t: option ;;\n+\t\t\t*=*)\t: setting env ;;\n+\t\t\tgit)\t: git itself ;;\n+\t\t\t\\(\\))   : skip parens of shell function definition ;;\n+\t\t\t{)\t: skip start of shell helper function ;;\n+\t\t\t:)\t: skip null command ;;\n+\t\t\t\\'*)\t: skip opening quote after sh -c ;;\n+\t\t\t*)\n+\t\t\t\tcur=\"$word\"\n+\t\t\t\tbreak\n+\t\t\tesac\n+\t\tdone\n \tdone\n+\n+\tif [[ \"$cur\" != \"$1\" ]]; then\n+\t\techo \"$cur\"\n+\tfi\n }\n \n # Check whether one of the given words is present on the command line,\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 4e943393cf..0e2db6e7fa 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2195,6 +2195,25 @@ test_expect_success 'complete files' '\n \ttest_completion \"git add mom\" \"momified\"\n '\n \n+test_expect_success \"simple alias\" '\n+\ttest_config alias.co checkout &&\n+\ttest_completion \"git co m\" <<-\\EOF\n+\tmaster Z\n+\tmybranch Z\n+\tmytag Z\n+\tEOF\n+'\n+\n+test_expect_success \"recursive alias\" '\n+\ttest_config alias.co checkout &&\n+\ttest_config alias.cod \"co --detached\" &&\n+\ttest_completion \"git cod m\" <<-\\EOF\n+\tmaster Z\n+\tmybranch Z\n+\tmytag Z\n+\tEOF\n+'\n+\n test_expect_success \"completion uses <cmd> completion for alias: !sh -c 'git <cmd> ...'\" '\n \ttest_config alias.co \"!sh -c '\"'\"'git checkout ...'\"'\"'\" &&\n \ttest_completion \"git co m\" <<-\\EOF\n-- \n2.29.2\n\n"},{"id":"409446","messageId":"20201110005331.532678-3-felipe.contreras@gmail.com","threadId":"54609","inReplyTo":"20201110005331.532678-1-felipe.contreras@gmail.com","subject":"[PATCH v2 2/3] completion: bash: check for alias loop","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-10T00:53:30Z","receivedAt":"2020-11-10T00:53:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We don't want to be stuck in an endless cycle.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 59ced25641..bf2a59f95e 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1121,11 +1121,17 @@ __git_pretty_aliases ()\n # __git_aliased_command requires 1 argument\n __git_aliased_command ()\n {\n-\tlocal cur=$1 last word cmdline\n+\tlocal cur=$1 last list word cmdline\n \n \twhile [[ \"$cur\" != \"$last\" ]]; do\n+\t\tif [[ \"$list\" == *\"$cur \"* ]]; then\n+\t\t\t# loop detected\n+\t\t\treturn\n+\t\tfi\n+\n \t\tcmdline=$(__git config --get \"alias.$cur\")\n \t\tlast=$cur\n+\t\tlist=\"$cur $list\"\n \n \t\tfor word in $cmdline; do\n \t\t\tcase \"$word\" in\n-- \n2.29.2\n\n"},{"id":"409447","messageId":"20201110005331.532678-4-felipe.contreras@gmail.com","threadId":"54609","inReplyTo":"20201110005331.532678-1-felipe.contreras@gmail.com","subject":"[PATCH v2 3/3] completion: bash: simplify __git_aliased_command","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-10T00:53:31Z","receivedAt":"2020-11-10T00:53:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex bf2a59f95e..ce0dc1e0f8 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1121,17 +1121,17 @@ __git_pretty_aliases ()\n # __git_aliased_command requires 1 argument\n __git_aliased_command ()\n {\n-\tlocal cur=$1 last list word cmdline\n+\tlocal cur=$1 list word cmdline\n \n-\twhile [[ \"$cur\" != \"$last\" ]]; do\n+\twhile [[ -n \"$cur\" ]]; do\n \t\tif [[ \"$list\" == *\"$cur \"* ]]; then\n \t\t\t# loop detected\n \t\t\treturn\n \t\tfi\n \n \t\tcmdline=$(__git config --get \"alias.$cur\")\n-\t\tlast=$cur\n \t\tlist=\"$cur $list\"\n+\t\tcur=\n \n \t\tfor word in $cmdline; do\n \t\t\tcase \"$word\" in\n@@ -1154,6 +1154,7 @@ __git_aliased_command ()\n \t\tdone\n \tdone\n \n+\tcur=\"${list%% *}\"\n \tif [[ \"$cur\" != \"$1\" ]]; then\n \t\techo \"$cur\"\n \tfi\n-- \n2.29.2\n\n"},{"id":"409455","messageId":"xmqqv9ee7zkx.fsf@gitster.c.googlers.com","threadId":"54609","inReplyTo":"20201110005331.532678-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 0/3] completion: bash: support for recursive/nested aliases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T01:47:10Z","receivedAt":"2020-11-10T01:47:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Hello,\n>\n> As Philippe reported, aliases of aliases are not completed correctly in the\n> Bash completion. This patch series attempts to fix that problem.\n>\n> Since v1:\n>\n>  * Use a loop instead of recursive calls\n>  * Add a check to detect alias loops\n>\n>\n> Felipe Contreras (3):\n>   completion: bash: support recursive aliases\n>   completion: bash: check for alias loop\n>   completion: bash: simplify __git_aliased_command\n\nIt is unclear why 3/3 needs to be separate (in other words, is there\na reason why 1/3 and 2/3 need to be done while the function is in\nthe more complex form, instead of doing what 1/3 and 2/3 wanted to\ndo to the function in a way that does not require later clean-up?),\nbut other than that, the end-result looks good.\n\nWill queue.\n"},{"id":"409459","messageId":"CAMP44s3qF2ie=+EgScyQtk_Z5qDeeWXBrJV61NTOkxOy6eBD-g@mail.gmail.com","threadId":"54609","inReplyTo":"xmqqv9ee7zkx.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 0/3] completion: bash: support for recursive/nested aliases","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-10T02:09:35Z","receivedAt":"2020-11-10T02:09:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 9, 2020 at 7:47 PM Junio C Hamano <gitster@pobox.com> wrote:\n\n> It is unclear why 3/3 needs to be separate (in other words, is there\n> a reason why 1/3 and 2/3 need to be done while the function is in\n> the more complex form, instead of doing what 1/3 and 2/3 wanted to\n> do to the function in a way that does not require later clean-up?),\n> but other than that, the end-result looks good.\n\nIt doesn't need to be separate. It's there just because the jump from\nthe current code to 1/3 is more natural this way (v2), but 3/3 can\ncertainly be merged to 1/3. We will have something a little bit odd in\nthe history:\n\n  cur=$last\n  if [[ \"$cur\" != \"$1\" ]]; then\n    echo \"$cur\"\n  fi\n\nWhy not just use $last instead of $cur? And in fact why clear $cur if\nwe are going to be setting it again?\n\nIt's just a little odd, but I sent a v3 with 1/3 and 3/3 merged, and\nit's not too odd.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}