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

> 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.

> 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 ...

> 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.

[Small typo correction that may affect how the message is read]

On Sat, Jun 20, 2026 at 9:18 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > 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.
>
> > 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 ...
>
> > 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

"is not" :)

> 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.
>


-- 
D. Ben Knoble

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

> [Small typo correction that may affect how the message is read]

Thanks, I spotted another one.

>> ... 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
>
> "is not" :)

True.

>> 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

"pretend" -> "present". 

>> 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?

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

> 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 ..

Will we see a hopefully small and final update (v4) to conclude this
topic anytime soom?  No rush, but just wondering...