{"thread":{"id":"54908","subject":"[PATCH v3 0/4] completion: make __git_complete public","startedAt":"2020-12-30T23:30:34Z","lastAt":"2020-12-30T23:30:51Z","messageCount":5,"participants":["Felipe Contreras"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"413204","messageId":"20201230232950.582152-1-felipe.contreras@gmail.com","threadId":"54908","inReplyTo":null,"subject":"[PATCH v3 0/4] completion: make __git_complete public","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-30T23:29:46Z","receivedAt":"2020-12-30T23:30:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Back in 2012 I argued [1] for the introduction of a helper that would\nallow users to specify aliases like:\n\n  git_complete gf git_fetch\n\nThere was pushback because there was no clear guideline for public\nfunctions (git_complete vs. _git_complete vs. _GIT_complete), and some\naliases didn't actually work.\n\nFast-forward to 2020, and there's still no guideline for public\nfunctions, and those aliases still don't work (even though I sent the\nfixes).\n\nThis has not prevented people from using this function that is clearly\nneeded to setup custom aliases [2], and in fact that's the recommended\nway since there's no other.\n\nBut it is cumbersome that the user must type:\n\n  __git_complete gf _git_fetch\n\nOr worse:\n\n  __git_complete gk __gitk_main\n\n8 years is more than enough time to stop waiting for the perfect to\ncome; let's define a public function (with the same name) that is\nactually user-friendly:\n\n  __git_complete gf git_fetch\n  __git_complete gk gitk\n\nWhile also maintaining backwards compatibility.\n\nChanges since v2:\n\n * Improved the safety of __git_have_func with suggestions from René Scharfe\n * Added documentation to the top of git-completion.bash\n\n[1] https://lore.kernel.org/git/1334524814-13581-1-git-send-email-felipe.contreras@gmail.com/\n[2] https://stackoverflow.com/questions/342969/how-do-i-get-bash-completion-to-work-with-aliases\n\nFelipe Contreras (4):\n  completion: bash: add __git_have_func helper\n  completion: bash: improve function detection\n  test: completion: add tests for __git_complete\n  completion: add proper public __git_complete\n\n contrib/completion/git-completion.bash | 50 ++++++++++++++++++++------\n t/t9902-completion.sh                  | 20 +++++++++++\n 2 files changed, 60 insertions(+), 10 deletions(-)\n\nRange-diff against v2:\n1:  0993732142 = 1:  0993732142 completion: bash: add __git_have_func helper\n-:  ---------- > 2:  6a3fe8b79b completion: bash: improve function detection\n2:  7918c34d0e = 3:  b211eebc0d test: completion: add tests for __git_complete\n3:  8a6cc52063 ! 4:  a5bfe5376b completion: add proper public __git_complete\n    @@ Commit message\n         Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n     \n      ## contrib/completion/git-completion.bash ##\n    +@@\n    + # tell the completion to use commit completion.  This also works with aliases\n    + # of form \"!sh -c '...'\".  For example, \"!sh -c ': git commit ; ... '\".\n    + #\n    ++# If you have a command that is not part of git, but you would still\n    ++# like completion, you can use __git_complete:\n    ++#\n    ++#   __git_complete gl git_log\n    ++#\n    ++# Or if it's a main command (i.e. git or gitk):\n    ++#\n    ++#   __git_complete gk gitk\n    ++#\n    + # Compatible with bash 3.2.57.\n    + #\n    + # You can set the following environment variables to influence the behavior of\n     @@ contrib/completion/git-completion.bash: __git_func_wrap ()\n      \t$1\n      }\n-- \n2.30.0\n\n"},{"id":"413205","messageId":"20201230232950.582152-3-felipe.contreras@gmail.com","threadId":"54908","inReplyTo":"20201230232950.582152-1-felipe.contreras@gmail.com","subject":"[PATCH v3 2/4] completion: bash: improve function detection","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-30T23:29:48Z","receivedAt":"2020-12-30T23:30:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":" 1. We should quote the argument\n 2. We don't need two redirections\n 3. A safeguard for arguments (-a) would be good\n\nSuggested-by: René Scharfe <l.s.r@web.de>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 869c73ee2c..1150d4bf44 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3359,7 +3359,7 @@ __git_support_parseopt_helper () {\n }\n \n __git_have_func () {\n-\tdeclare -f $1 >/dev/null 2>/dev/null\n+\tdeclare -f -- \"$1\" >/dev/null 2>&1\n }\n \n __git_complete_command () {\n-- \n2.30.0\n\n"},{"id":"413206","messageId":"20201230232950.582152-2-felipe.contreras@gmail.com","threadId":"54908","inReplyTo":"20201230232950.582152-1-felipe.contreras@gmail.com","subject":"[PATCH v3 1/4] completion: bash: add __git_have_func helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-30T23:29:47Z","receivedAt":"2020-12-30T23:30:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This makes the code more readable, and also will help when new code\nwants to do similar checks.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 463a3124da..869c73ee2c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3358,15 +3358,19 @@ __git_support_parseopt_helper () {\n \tesac\n }\n \n+__git_have_func () {\n+\tdeclare -f $1 >/dev/null 2>/dev/null\n+}\n+\n __git_complete_command () {\n \tlocal command=\"$1\"\n \tlocal completion_func=\"_git_${command//-/_}\"\n-\tif ! declare -f $completion_func >/dev/null 2>/dev/null &&\n-\t\tdeclare -f _completion_loader >/dev/null 2>/dev/null\n+\tif ! __git_have_func $completion_func &&\n+\t\t__git_have_func _completion_loader\n \tthen\n \t\t_completion_loader \"git-$command\"\n \tfi\n-\tif declare -f $completion_func >/dev/null 2>/dev/null\n+\tif __git_have_func $completion_func\n \tthen\n \t\t$completion_func\n \t\treturn 0\n-- \n2.30.0\n\n"},{"id":"413207","messageId":"20201230232950.582152-4-felipe.contreras@gmail.com","threadId":"54908","inReplyTo":"20201230232950.582152-1-felipe.contreras@gmail.com","subject":"[PATCH v3 3/4] test: completion: add tests for __git_complete","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-30T23:29:49Z","receivedAt":"2020-12-30T23:30:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Even though the function was marked as not public, it's already used in\nthe wild.\n\nWe should at least test basic functionality.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex a1c4f1f6d4..c0b4380eae 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2380,4 +2380,12 @@ test_expect_success 'sourcing the completion script clears cached --options' '\n \tverbose test -z \"$__gitcomp_builtin_notes_edit\"\n '\n \n+test_expect_success '__git_complete' '\n+\tunset -f __git_wrap__git_main &&\n+\t__git_complete foo __git_main &&\n+\t__git_have_func __git_wrap__git_main &&\n+\t__git_complete gf _git_fetch &&\n+\t__git_have_func __git_wrap_git_fetch\n+'\n+\n test_done\n-- \n2.30.0\n\n"},{"id":"413208","messageId":"20201230232950.582152-5-felipe.contreras@gmail.com","threadId":"54908","inReplyTo":"20201230232950.582152-1-felipe.contreras@gmail.com","subject":"[PATCH v3 4/4] completion: add proper public __git_complete","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-30T23:29:50Z","receivedAt":"2020-12-30T23:30:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"When __git_complete was introduced, it was meant to be temporarily, while\na proper guideline for public shell functions was established\n(tentatively _GIT_complete), but since that never happened, people\nin the wild started to use __git_complete, even though it was marked as\nnot public.\n\nEight years is more than enough wait, let's mark this function as\npublic, and make it a bit more user-friendly.\n\nSo that instead of doing:\n\n  __git_complete gk __gitk_main\n\nThe user can do:\n\n  __git_complete gk gitk\n\nAnd instead of:\n\n  __git_complete gf _git_fetch\n\nDo:\n\n  __git_complete gf git_fetch\n\nBackwards compatibility is maintained.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 40 +++++++++++++++++++++-----\n t/t9902-completion.sh                  | 14 ++++++++-\n 2 files changed, 46 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 1150d4bf44..4b1f4264a6 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -29,6 +29,15 @@\n # tell the completion to use commit completion.  This also works with aliases\n # of form \"!sh -c '...'\".  For example, \"!sh -c ': git commit ; ... '\".\n #\n+# If you have a command that is not part of git, but you would still\n+# like completion, you can use __git_complete:\n+#\n+#   __git_complete gl git_log\n+#\n+# Or if it's a main command (i.e. git or gitk):\n+#\n+#   __git_complete gk gitk\n+#\n # Compatible with bash 3.2.57.\n #\n # You can set the following environment variables to influence the behavior of\n@@ -3497,10 +3506,7 @@ __git_func_wrap ()\n \t$1\n }\n \n-# Setup completion for certain functions defined above by setting common\n-# variables and workarounds.\n-# This is NOT a public function; use at your own risk.\n-__git_complete ()\n+___git_complete ()\n {\n \tlocal wrapper=\"__git_wrap${2}\"\n \teval \"$wrapper () { __git_func_wrap $2 ; }\"\n@@ -3508,13 +3514,33 @@ __git_complete ()\n \t\t|| complete -o default -o nospace -F $wrapper $1\n }\n \n-__git_complete git __git_main\n-__git_complete gitk __gitk_main\n+# Setup the completion for git commands\n+# 1: command or alias\n+# 2: function to call (e.g. `git`, `gitk`, `git_fetch`)\n+__git_complete ()\n+{\n+\tlocal func\n+\n+\tif __git_have_func $2; then\n+\t\tfunc=$2\n+\telif __git_have_func __$2_main; then\n+\t\tfunc=__$2_main\n+\telif __git_have_func _$2; then\n+\t\tfunc=_$2\n+\telse\n+\t\techo \"ERROR: could not find function '$2'\" 1>&2\n+\t\treturn 1\n+\tfi\n+\t___git_complete $1 $func\n+}\n+\n+___git_complete git __git_main\n+___git_complete gitk __gitk_main\n \n # The following are necessary only for Cygwin, and only are needed\n # when the user has tab-completed the executable name and consequently\n # included the '.exe' suffix.\n #\n if [ \"$OSTYPE\" = cygwin ]; then\n-\t__git_complete git.exe __git_main\n+\t___git_complete git.exe __git_main\n fi\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex c0b4380eae..c4a7758409 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2382,10 +2382,22 @@ test_expect_success 'sourcing the completion script clears cached --options' '\n \n test_expect_success '__git_complete' '\n \tunset -f __git_wrap__git_main &&\n+\n \t__git_complete foo __git_main &&\n \t__git_have_func __git_wrap__git_main &&\n+\tunset -f __git_wrap__git_main &&\n+\n \t__git_complete gf _git_fetch &&\n-\t__git_have_func __git_wrap_git_fetch\n+\t__git_have_func __git_wrap_git_fetch &&\n+\n+\t__git_complete foo git &&\n+\t__git_have_func __git_wrap__git_main &&\n+\tunset -f __git_wrap__git_main &&\n+\n+\t__git_complete gd git_diff &&\n+\t__git_have_func __git_wrap_git_diff &&\n+\n+\ttest_must_fail __git_complete ga missing\n '\n \n test_done\n-- \n2.30.0\n\n"}]}