[PATCH v2 0/3] completion: make __git_complete public

[Felipe Contreras <felipe.contreras@gmail.com>]

Back in 2012 I argued [1] for the introduction of a helper that would
allow users to specify aliases like:

  git_complete gf git_fetch

There was pushback because there was no clear guideline for public
functions (git_complete vs. _git_complete vs. _GIT_complete), and some
aliases didn't actually work.

Fast-forward to 2020, and there's still no guideline for public
functions, and those aliases still don't work (even though I sent the
fixes).

This has not prevented people from using this function that is clearly
needed to setup custom aliases [2], and in fact that's the recommended
way since there's no other.

But it is cumbersome that the user must type:

  __git_complete gf _git_fetch

Or worse:

  __git_complete gk __gitk_main

8 years is more than enough time to stop waiting for the perfect to
come; let's define a public function (with the same name) that is
actually user-friendly:

  __git_complete gf git_fetch
  __git_complete gk gitk

While also maintaining backwards compatibility.

[1] https://lore.kernel.org/git/1334524814-13581-1-git-send-email-felipe.contreras@gmail.com/
[2] https://stackoverflow.com/questions/342969/how-do-i-get-bash-completion-to-work-with-aliases

Felipe Contreras (3):
  completion: bash: add __git_have_func helper
  test: completion: add tests for __git_complete
  completion: add proper public __git_complete

 contrib/completion/git-completion.bash | 41 +++++++++++++++++++-------
 t/t9902-completion.sh                  | 20 +++++++++++++
 2 files changed, 51 insertions(+), 10 deletions(-)

Range-diff:
-:  ---------- > 1:  0993732142 completion: bash: add __git_have_func helper
1:  ea77b1a140 ! 2:  7918c34d0e test: completion: add tests for __git_complete
    @@ Metadata
      ## Commit message ##
         test: completion: add tests for __git_complete
     
    +    Even though the function was marked as not public, it's already used in
    +    the wild.
    +
    +    We should at least test basic functionality.
    +
         Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
     
      ## t/t9902-completion.sh ##
    @@ t/t9902-completion.sh: test_expect_success 'sourcing the completion script clear
      '
      
     +test_expect_success '__git_complete' '
    ++	unset -f __git_wrap__git_main &&
     +	__git_complete foo __git_main &&
    -+	test "$(type -t __git_wrap__git_main)" == function &&
    ++	__git_have_func __git_wrap__git_main &&
     +	__git_complete gf _git_fetch &&
    -+	test "$(type -t __git_wrap_git_fetch)" == function
    ++	__git_have_func __git_wrap_git_fetch
     +'
     +
      test_done
2:  aec19e61ee ! 3:  8a6cc52063 completion: add proper public __git_complete
    @@ Metadata
      ## Commit message ##
         completion: add proper public __git_complete
     
    -    Back in 2012 I argued [1] for the introduction of a helper that would
    -    allow users to specify aliases like:
    +    When __git_complete was introduced, it was meant to be temporarily, while
    +    a proper guideline for public shell functions was established
    +    (tentatively _GIT_complete), but since that never happened, people
    +    in the wild started to use __git_complete, even though it was marked as
    +    not public.
     
    -      git_complete gf git_fetch
    +    Eight years is more than enough wait, let's mark this function as
    +    public, and make it a bit more user-friendly.
     
    -    Back then there was pushback because there was no clear guideline for
    -    public functions (git_complete vs _git_complete vs _GIT_complete), and
    -    some aliases didn't actually work.
    -
    -    Fast-forward to 2020 and there's still no guideline for public
    -    functions, and those aliases still don't work (even though I sent the
    -    fixes).
    -
    -    This has not prevented people from using this function that is clearly
    -    needed to setup custom aliases [2], and in fact it's the recommended
    -    way. But it is cumbersome that the user must type:
    -
    -      __git_complete gf _git_fetch
    -
    -    Or worse:
    +    So that instead of doing:
     
           __git_complete gk __gitk_main
     
    -    8 years is more than enough time to stop waiting for the perfect to
    -    come; let's define a public function (with the same name) that is
    -    actually user-friendly:
    +    The user can do:
     
    -      __git_complete gf git_fetch
           __git_complete gk gitk
     
    -    While also maintaining backwards compatibility.
    +    And instead of:
     
    -    The logic is:
    +      __git_complete gf _git_fetch
    +
    +    Do:
     
    -     1. If $2 exists, use it directly
    -     2. If not, check if __$2_main exists
    -     3. If not, check if _$2 exists
    -     4. If not, fail
    +      __git_complete gf git_fetch
     
    -    [1] https://lore.kernel.org/git/1334524814-13581-1-git-send-email-felipe.contreras@gmail.com/
    -    [2] https://stackoverflow.com/questions/342969/how-do-i-get-bash-completion-to-work-with-aliases
    +    Backwards compatibility is maintained.
     
         Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
     
    @@ contrib/completion/git-completion.bash: __git_complete ()
     +{
     +	local func
     +
    -+	if [[ "$(type -t $2)" == function ]]; then
    ++	if __git_have_func $2; then
     +		func=$2
    -+	elif [[ "$(type -t __$2_main)" == function ]]; then
    ++	elif __git_have_func __$2_main; then
     +		func=__$2_main
    -+	elif [[ "$(type -t _$2)" == function ]]; then
    ++	elif __git_have_func _$2; then
     +		func=_$2
     +	else
     +		echo "ERROR: could not find function '$2'" 1>&2
    @@ contrib/completion/git-completion.bash: __git_complete ()
     
      ## t/t9902-completion.sh ##
     @@ t/t9902-completion.sh: test_expect_success 'sourcing the completion script clears cached --options' '
    - '
      
      test_expect_success '__git_complete' '
    -+	unset -f __git_wrap__git_main &&
    + 	unset -f __git_wrap__git_main &&
     +
      	__git_complete foo __git_main &&
    - 	test "$(type -t __git_wrap__git_main)" == function &&
    + 	__git_have_func __git_wrap__git_main &&
     +	unset -f __git_wrap__git_main &&
     +
      	__git_complete gf _git_fetch &&
    --	test "$(type -t __git_wrap_git_fetch)" == function
    -+	test "$(type -t __git_wrap_git_fetch)" == function &&
    +-	__git_have_func __git_wrap_git_fetch
    ++	__git_have_func __git_wrap_git_fetch &&
     +
     +	__git_complete foo git &&
    -+	test "$(type -t __git_wrap__git_main)" == function &&
    ++	__git_have_func __git_wrap__git_main &&
     +	unset -f __git_wrap__git_main &&
     +
     +	__git_complete gd git_diff &&
    -+	test "$(type -t __git_wrap_git_diff)" == function &&
    ++	__git_have_func __git_wrap_git_diff &&
     +
     +	test_must_fail __git_complete ga missing
      '
-- 
2.30.0