Re: [PATCH v3 0/2] completion: hide dotfiles for selected path completion

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 21, 2026, 01:17 UTC
Message-ID: <xmqq1pe0g08t.fsf@gitster.g>
In-Reply-To: <pull.2311.v3.git.git.1781978156.gitgitgadget@gmail.com>

"Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 10 quoted lines

> The completion helper for index paths uses git ls-files rather than shell
> filename completion. As a result, leading-dot paths such as a tracked
> .gitignore were offered even when the user had not started the path with ..
>
> Hide leading-dot path components for git rm, git mv, and git ls-files when
> completing an empty path component. Explicit dot completion is still
> preserved, so git rm . can still complete .gitignore.
>
> This removes the existing TODO expectations in t/t9902-completion.sh and
> adds coverage for explicit dot completion.

OK.

Show 6 quoted lines

> Validation:
>
>  * git diff --check -- contrib/completion/git-completion.bash
>    t/t9902-completion.sh
>  * bash -n contrib/completion/git-completion.bash
>  * ./t9902-completion.sh

I am not sure what you wanted to say with these lines. If you did the above to build confidence that your patch works, that would be great. Or are you telling readers to do these things and when they do not see any issues consider your patch perfect?

What is missing around here in this cover letter is a description of how this iteration is different from the previous one. And ...

Show 18 quoted lines

> Zakariyah Ali (2):
>   completion: hide dotfiles for selected path completion
>   completion: hide dotfiles by default for path completion
>
>  contrib/completion/git-completion.bash | 53 +++++++++++++++-----------
>  t/t9902-completion.sh                  | 19 ++++-----
>  2 files changed, 40 insertions(+), 32 deletions(-)
>
>
> base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v3
> Pull-Request: https://github.com/git/git/pull/2311
>
> Range-diff vs v2:
>
>  1:  056e239e06 = 1:  056e239e06 completion: hide dotfiles for selected path completion
>  -:  ---------- > 2:  7482ee4645 completion: hide dotfiles by default for path completion

... I find this range diff very troubling. If we look at patch 2, it seems that it redoes some part of what is done in patch 1 saying "oops that was wrong, so let's do it better this time". Such a drunken-mans' walk that goes in one direction in an earlier step, only to be corrected to move to a different course, is now how we want a new topic to be presented.

The end result may be much easier to read, mostly thanks to updated loop in the awk script, so if we really want to pretend this as two patches for "small pieces are easier to digest" value, perhaps have [PATCH 1/2] that updates the awk script (without doing anything related to hide-dotfiles theme) to make it easier to read by not having multiple "print pfx p" in it, and then build on top of that improved base, have [PATCH 2/2] that adds the support to hide dotfiles, perhaps?

Since the initial iteration was quite a while ago, I no longer remember the details of the review I gave, but I recall having hard time telling which callers of the complete-index-file helper hide dotfiles from their output and which callers do not hide them, and how the patch decided to choose which ones should and should not hide. Has it been improved and if so how? That is something we expect the cover letter to tell, too.

Thanks.