Thank you both for your input. Answers inline below!
Den lör 1 jan. 2022 kl 17:16 skrev Daniel Shahaf <d...@daniel.shahaf.name>:
> Nathan Hartman wrote on Fri, 31 Dec 2021 00:01 +00:00:
> > On Tue, Dec 28, 2021 at 11:19 AM Daniel Sahlberg
> > <daniel.l.sahlb...@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >> While digging through Jira I stumbled upon the above mentioned issue.
> >>
> >> It seems fairly simple to add the -o bashdefault option when defining
> the completion.
> >>
> >> Patch attached, anyone want to comment before I commit?
> >>
> >> Kind regards,
> >> Daniel
> >
> >
> > Interesting! I'm not an expert on bash completion but I tried the
> > patch and I can confirm it seems to enable this functionality on my
> > system.
> >
> > I am slightly worried, though, that "-o bashdefault" might be a
> > bashism not compatible with all systems or shells.
>
> Isn't tools/client-side/bash_completion specific to bash?
>
The script says:
[[[
Known to work with bash 3.* with programmable completion
]]]
This was changed in r877553 in 2009 when svndumpfilter was added. I don't
know if it was required or if it was changed because bash 3 (released in
2004) at that time was mature/common.
I've checked and according to the bash CHANGES file, bashdefault was added
in bash 3. So by adding the option we are not even incrementing the
required bash version from what is already documented.
> I can't speak to "all systems or shells", but I can answer for zsh. zsh
> has a native svn(1) completion in [1], as well as an emulation facility
> for loading bash completion functions ("bashcompinit").
>
> Users are likely to be using the former.
>
> Using our bash completion under zsh via bashcompinit is… well, not
> inconceivable, and not impossible, but it requires manually changing
> tools/client-side/bash_completion to work around a difference between
> the two shells' syntaxes, and after that there are codepaths that raise
> runtime errors, some of which warrant further code changes (e.g., in zsh
> $status is read- only). In short, I think it's safe to assume zsh users
> don't use our bash completion.
>
> [1]
> https://github.com/zsh-users/zsh/blob/master/Completion/Unix/Command/_subversion
>
> > Since the bug
> > report mentioned git and mplayer, I perused the completion scripts for
> > those programs on my debian box to see what they're doing. Strangely
> > the mplayer one didn't seem to have either "-o bashdefault" or "-o
> > default", but it contains a lot of logic I haven't tried to grok;
> > meanwhile the git one, also with a great deal of other logic I haven't
> > tried to grok, seems to try using both options together "-o
> > bashdefault -o default" and, if that fails, tries with just "-o
> > default", like this:
> >
> > complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null
> || complete -o default -o nospace -F $wrapper $1
>
> I'm just guessing, but it could be that «bashdefault» was a recent
> addition to bash at the time that line was written, so the completion
> tried to support older versions of bash as well. (That's always
> a problem with completion functions: they depend both on the shell's
> version and on the completed program's version.)
>
> Or perhaps «bashdefault» was added to bash by a distro patch, and git's
> completion tries to be portable to other vendors.
>
I've been digging around in git's repository and it seems that this is in
the upsteam script and not a distro patch.
The actual code was added 13 years ago [1] , with a commit message [[[ ...
adds "-o bashdefault" to the completion line. If that option is not
available, it uses the old method. This behaviour was inspired by
Mercurial's bash completion script. ]]]
At that time it was probably not unreasonable to think that someone was
running bash 2, and that the fallback was required.
I'm not convinced that it is required today but I think the idea is quite
clever.
P.S. Issue #4714 is also about bash completions.
>
Thanks for raising this. Unfortunately it doesn't really match my current
filter of low-hanging fruits but I will try to remember to circle back
later.
Kind regards,
Daniel Sahlberg
[1]
https://github.com/git/git/commit/50e126e185c196b9b66dcdefb4b05f090d62dd4c
