You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Sahlberg <da...@gmail.com> on 2021/12/28 16:19:09 UTC
[PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
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
Re: [PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Dec 30, 2021 at 7:01 PM Nathan Hartman <ha...@gmail.com> wrote:
> complete -o bashdefault -o default -o nospace -F $wrapper $1
> 2>/dev/null || complete -o default -o nospace -F $wrapper $1
The above automatically wrapped but should be one line.
Nathan
Re: [PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
Posted by Daniel Sahlberg <da...@gmail.com>.
Den sön 2 jan. 2022 kl 04:36 skrev Nathan Hartman <hartman.nathan@gmail.com
>:
> On Sat, Jan 1, 2022 at 4:44 PM Daniel Sahlberg
> <da...@gmail.com> wrote:
> >
> > Thank you both for your input. Answers inline below!
> >
> > Den lör 1 jan. 2022 kl 17:16 skrev Daniel Shahaf <d.s@daniel.shahaf.name
> >:
> >>
> >> Nathan Hartman wrote on Fri, 31 Dec 2021 00:01 +00:00:
> (snip)
> >> > 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.
>
>
> Well, the above pretty much answers my concerns, so I'm +1 to commit
> and close out this issue.
>
Thanks, I've committed the patch as r1896619. I think I saw somewhere that
issues are usually kept open until the fix has been shipped.
Kind regards,
Daniel
Re: [PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
Posted by Nathan Hartman <ha...@gmail.com>.
On Sat, Jan 1, 2022 at 4:44 PM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> 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:
(snip)
>> > 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.
Well, the above pretty much answers my concerns, so I'm +1 to commit
and close out this issue.
Happy New Year,
Nathan
Re: [PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
Posted by Daniel Sahlberg <da...@gmail.com>.
Den lör 1 jan. 2022 kl 22:44 skrev Daniel Sahlberg <
daniel.l.sahlberg@gmail.com>:
> 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
>> > <da...@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
>
>
Oh, I forgot to mention. I also investigated tcsh which has programmable
completion (using the "complete" command, just like bash). However it has a
completely different syntax. Git has an interesting solution: It creates a
minimal script that calls the bash-completion script in bash.
/Daniel
Re: [PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
Posted by Daniel Sahlberg <da...@gmail.com>.
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
> > <da...@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
Re: [PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
Posted by 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
> <da...@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?
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.
Daniel
P.S. Issue #4714 is also about bash completions.
Re: [PATCH] Jira SVN-3714 Tab completion does not expand ~username directory
Posted by Nathan Hartman <ha...@gmail.com>.
On Tue, Dec 28, 2021 at 11:19 AM Daniel Sahlberg
<da...@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. 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
Cheers,
Nathan