Re: [PATCH v4 2/3] completion: complete tracked paths for 'git diff'

>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Thu, Aug 6, 2026 at 6:38 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> 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'.
> >
> > I'm looking forward to using this.  :-)
> >
> > [...]
> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> >> index ccd3b2a372..845fd19f70 100644
> >> --- a/contrib/completion/git-completion.bash
> >> +++ b/contrib/completion/git-completion.bash
> >> @@ -1981,6 +1981,10 @@ _git_diff ()
> >>                 esac
> >>                 __git_complete_revlist_file
> >>         fi
> >> +
> >> +       if [ ${#COMPREPLY[@]} -eq 0 ]; then
> >> +               __git_complete_index_file
> >> +       fi
> >>  }
> >
> > Curious; __git_complete_index_file() is documented as "requires 1
> > argument", but you pass none here.  As far as I can tell, it works
> > anyway, but feels like an accident:
> >
> > 1.   __git_complete_index_file CALLS
> >       __git_index_files "$1" ...
> >       (Here, "$1" == "")
> > 2.   __git_index_files "$1" ... CALLS
> >       __git_ls_files_helper "$root" "$1" ...
> >       (Here, "$1" == "", again)
> > 3.   __git_ls_files_helper "$root" "$1" CALLS
> >       __git -C "$1" -c core.quotePath=false ls-files
> > --exclude-standard $2 -- ...
> >       (Note that $2 is unquoted, and since it's empty, it disappears)
> >
> > It seems like it'd be better to pass an explicit "" to
> > __git_complete_index_file than to implicitly get it.
>
> OK.  It feels a bit strange as an API for the function to insist
> taking one and only one option, which forces the caller to do
>
>         __git_complete_index_file "--cached --others --directory"
>
> when the intention clearly is "we take zero or more options that we
> pass to ls-files", which would have been more obvious if the above
> were written as three separate parameters, but I'll do as Romans in
> the (hopefully small and final) reroll.
>
> Thanks.