[PATCH v2] completion: complete tracked paths for 'git diff'

[Junio C Hamano <gitster@pobox.com>]

When completing arguments for 'git diff', _git_diff() delegates to
__git_complete_revlist_file(), which only completes revision
references.  This is good [*], as mixing both revisions and paths in a
single list for the user to pick from is simply too confusing.

If no reference matches, or if '--' is given, however, _git_diff()
leaves COMPREPLY empty.  Bash then falls back to default filename
completion in $PWD.  This fails when 'git -C <path>' is used because
$PWD is not the target repository.

Update _git_diff() to use __git_complete_index_file() when '--' is
present, or when revision reference completion yields no matching
candidates, so that tracked paths are offered as candidates.

This changes behavior even in the case where '-C <there>' is not
used.  The new behavior omits untracked paths from suggestions when
no revs match the prefix but matching tracked paths exist, which is
more useful in the context of 'git diff'.

When run outside the working tree of a repository, or when nothing
matches from revisions or tracked paths, Bash still falls back to
default filename completion in $PWD, so such a use case would be
just like completing paths for any 'diff' command, rather than for
'git diff'.

[Footnote]
 * In https://lore.kernel.org/git/al%2Fw2qgBfhe9qMg6@szeder.dev/
   SZEDER made the same argument for "git send-email 0<TAB>".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * The last two paragraphs in the proposed commit log message are
   new, to explain why the code posted as-is would be sufficient to
   support the "'git diff --no-index' is not Git but is diff" usage,
   and there is no code change between v1 and this iteration.

   By the way, I, as a relative newbie to the completion script, had
   trouble with the test_completion helper and wasted some time
   wondering why an additional test:

    test_expect_success 'git diff completes untracked paths if  nothing matches' '
	    >untracked &&
	    test_completion "git diff -- u" <<-\EOF
	    untracked
	    EOF
    '

    did not work, even though under manual testing, u<TAB> completed
    'untracked' just fine.  The reason is that test_completion
    does not test the final "Bash default" fallback.  It might not
    be necessary for those who are familiar with the completion test
    suite, but I thought it would help others.

    This message comes with a range-diff that shows only the commit
    log changes.

 contrib/completion/git-completion.bash |  8 +++++-
 t/t9902-completion.sh                  | 40 ++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e875787710..8f5773292b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1947,7 +1947,10 @@ __git_diff_difftool_options="--cached --staged
 
 _git_diff ()
 {
-	__git_has_doubledash && return
+	if __git_has_doubledash; then
+		__git_complete_index_file
+		return
+	fi
 
 	case "$cur" in
 	--diff-algorithm=*)
@@ -1976,6 +1979,9 @@ _git_diff ()
 		;;
 	esac
 	__git_complete_revlist_file
+	if [ ${#COMPREPLY[@]} -eq 0 ]; then
+		__git_complete_index_file
+	fi
 }
 
 __git_mergetools_common="diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 9ae3c48ebd..82488f3b50 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2663,6 +2663,7 @@ test_expect_success 'setup for integration tests' '
 	echo content >file1 &&
 	echo more >file2 &&
 	git add file1 file2 &&
+	echo untracked >file3 &&
 	git commit -m one &&
 	git branch mybranch &&
 	git tag mytag
@@ -2712,6 +2713,45 @@ test_expect_success 'git -C <path> checkout uses the right repo' '
 	EOF
 '
 
+test_expect_success 'git diff completes tracked paths when no refs match' '
+	# file1 and file2 are tracked but file3 is not
+	test_completion "git diff f" <<-\EOF
+	file1
+	file2
+	EOF
+'
+
+test_expect_success 'git diff -- completes tracked paths' '
+	# file1 and file2 are tracked but file3 is not
+	test_completion "git diff -- f" <<-\EOF
+	file1
+	file2
+	EOF
+'
+
+test_expect_success 'git -C <path> diff completes tracked paths in specified repo' '
+	test_when_finished "rm -rf repo-for-diff" &&
+	git init repo-for-diff &&
+	echo content >repo-for-diff/otherfile &&
+	git -C repo-for-diff add otherfile &&
+	echo untracked >repo-for-diff/oops &&
+	git -C repo-for-diff commit -m otherfile &&
+	test_completion "git -C repo-for-diff diff o" <<-\EOF
+	otherfile
+	EOF
+'
+
+test_expect_success 'git -C <path> diff -- completes pathspecs in specified repo' '
+	test_when_finished "rm -rf repo-for-diff" &&
+	git init repo-for-diff &&
+	echo content >repo-for-diff/otherfile &&
+	git -C repo-for-diff add otherfile &&
+	git -C repo-for-diff commit -m otherfile &&
+	test_completion "git -C repo-for-diff diff -- o" <<-\EOF
+	otherfile
+	EOF
+'
+
 test_expect_success 'show completes all refs' '
 	test_completion "git show m" <<-\EOF
 	main Z

Range-diff against v1:
1:  100043822f ! 1:  fa4461a192 completion: complete tracked paths for 'git diff'
    @@ Commit message
     
         When completing arguments for 'git diff', _git_diff() delegates to
         __git_complete_revlist_file(), which only completes revision
    -    references.  This is good [*], as mixing both revs and paths in a
    -    single list to have the user pick is simply too confusing.
    +    references.  This is good [*], as mixing both revisions and paths in a
    +    single list for the user to pick from is simply too confusing.
     
         If no reference matches, or if '--' is given, however, _git_diff()
    -    leaves COMPREPLY empty. Bash then falls back to default filename
    -    completion in $PWD. This fails when 'git -C <path>' is used because
    +    leaves COMPREPLY empty.  Bash then falls back to default filename
    +    completion in $PWD.  This fails when 'git -C <path>' is used because
         $PWD is not the target repository.
     
         Update _git_diff() to use __git_complete_index_file() when '--' is
         present, or when revision reference completion yields no matching
         candidates, so that tracked paths are offered as candidates.
     
    -    [Footnote]
    +    This changes behavior even in the case where '-C <there>' is not
    +    used.  The new behavior omits untracked paths from suggestions when
    +    no revs match the prefix but matching tracked paths exist, which is
    +    more useful in the context of 'git diff'.
    +
    +    When run outside the working tree of a repository, or when nothing
    +    matches from revisions or tracked paths, Bash still falls back to
    +    default filename completion in $PWD, so such a use case would be
    +    just like completing paths for any 'diff' command, rather than for
    +    'git diff'.
     
    +    [Footnote]
          * In https://lore.kernel.org/git/al%2Fw2qgBfhe9qMg6@szeder.dev/
            SZEDER made the same argument for "git send-email 0<TAB>".
     
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
    +    ---
    +
    +     * The last two paragraphs in the proposed commit log message are
    +       new, to explain why the code posted as-is would be sufficient to
    +       support the "'git diff --no-index' is not Git but is diff" usage.
    +       I, as a relative newbie to the completion script, had trouble
    +       with the test_completion helper and wasted some time wondering
    +       why an additional test:
    +
    +        test_expect_success 'git diff completes untracked paths if  nothing matches' '
    +                >untracked &&
    +                test_completion "git diff -- u" <<-\EOF
    +                untracked
    +                EOF
    +        '
    +
    +        did not work, even though under manual testing, u<TAB> completed
    +        'untracked' just fine.  The reason is that test_completion
    +        does not test the final "Bash default" fallback.  It might not
    +        be necessary for those who are familiar with the completion test
    +        suite, but I thought it would help others.
     
      ## contrib/completion/git-completion.bash ##
     @@ contrib/completion/git-completion.bash: __git_diff_difftool_options="--cached --staged
-- 
2.55.0-624-gcdeb5fd34c