Re: [PATCH 3/3] completion: fix bugs with slashes in remote names

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 3, 2025 at 2:25 PM David Mandelberg <david@xxxxxxxxxxxxxx> wrote:
>
> Op 2025-03-03 om 11:36 schreef phillip.wood123@xxxxxxxxx:
> >>> local sed_cmd=
> >>> local remote
> >>> # ref names and therefore remote names cannot contain '*?[]^' so we
> >>> # only need to escape '.$/'. Using 'sort -r' means that if there is a
> >>> # remote called "github" and another called "github/alice" we will try
> >>> # and strip "github/alice" first.
> >>> for remote in $(__git_remotes | sort -r)
> >>> do
> >>>      remote="${remote//./\\./}"
> >>>      remote="${remote//\$/\\\$/}"
> >>>      remote="${remote//\//\\\//}"
> >>
> >> Just FYI since it took me hours to figure this out myself: I think
> >> this would break tests on macos because of an old version of bash that
> >> handles backslashes weirdly. I think removing the double quotes would
> >> work around that issue, and be safe because word splitting doesn't
> >> happen in assignments.
> >
> > Thanks, I'm not familiar with bash's extensions to parameter
> > substitution. The completions can also but used under zsh (git-
> > completion.zsh is pretty much abandoned I think) but it looks like bash
> > and zsh agree on this expansion.

I don't think "abandoned" is the right characterization—at least, it's
the completion script that Homebrew-installed Git users will get for
Zsh, and it does re-use the Bash completion scripts (which therefore
ought to stay portable between both if possible, otherwise we get
stuff like 8776470cf3 (completion: repair config completion for Zsh,
2025-01-06)). The Zsh script has seen some fixups over time (such as
my own 3c20acdf46 (completion: zsh: stop leaking local cache variable,
2024-04-30)).

>
> The tests don't automatically run under zsh though, right? Maybe I
> should try to figure out how to do that in a separate patch...

Correct—which is how bugs sneak in ;)

I'm willing to manually test the patch if I can understand how to
reproduce the issue—it sounds like having a remote name with a slash
is sufficient?

I started trying to test Zsh completion a while back when working on
one of the patches mentioned above; I got as far as this hack [1],
which is to say, not very far at all.

[1]: https://github.com/git/git/commit/d8918195f18a503aa1a42fed3c66a0af8d04131f

--
D. Ben Knoble





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux