Re: [PATCH] commit: correctly escape @ of stashes

> Ok, then I think it's best if I just close this PR. I didn't know that it
> worked alright in newer versions of BASH. And I haven't found a way to make
> it work without the escaping of @. It seems that the patch doesn't bring
> any value to the community after all...
>
> Cheers and thanks once more for the review and insights!
>
> Best wishes,
> Martin

>
> On Wed, Jul 8, 2020, 23:54 Junio C Hamano <gitster@pobox.com> wrote:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > >> In fact, I just tried
> > >>
> > >>     $ git stash show stas<TAB>
> > >>
> > >> in a test repository where there is only one stash entry and got it
> > >> completed to
> > >>
> > >>     $ git stash show stash@{0}
> > >>
> > >> and pressing <Enter> from this state did exactly what I expected to
> > >> see.
> > >>
> > >> > Reproducible on `GNU bash, version 3.2.57(1)-release` and
> > >> > `macOS Catalina 10.15.5`.
> > >>
> > >> What did you reproduce?  The completion gave me "stash@{0}" (without
> > >> surrounding double quotes) in my above experiment?  If so, that does
> > >> seem reproducible, but I do not see "suggestions ... are broken" here.
> > >
> > > The problem is visible when you have more than one stash. If
> > > you press TABm autocompletion stops at the `{` and if you then
> > > press TAB once more for a list of all stashes instead of that
> > > the completion changes to `stashstash{` and becomes broken at
> > > this point.
> >
> > That does not reproduce for me.
> >
> >     $ git stash show stas<TAB>
> >
> > completes to
> >
> >     $ git stash show stash@{
> >
> > and any more <TAB> changes the command line, which is what I expect,
> > as it would be stupid to offer stash@{0} and stash@{1} (and others)
> > as possible completion candidates.
> >
> > Also I just tried this (manually)
> >
> >     $ git stash show stash\@{<TAB>
> >
> > and did not get any more extra completion.
> >
> >     $ set | grep ^BASH_VERSION=
> >     BASH_VERSION='5.0.16(1)-release'
> >
> > As long as the change does not make things worse for users of newer
> > bash, it is OK to make things better for users of ancient versions
> > of bash.  It might already be considered a worse end-user experience
> > than what we have now to show partly-spelled stash reference as
> > "stash\@{" with backslash, though.
> >
> > Another thing I noticed is that you shouldn't need two separate
> > processes of "sed" to do what you want to do.
> >
> > >> > @@ -2999,12 +2999,14 @@ _git_stash ()
> > >> >                            __git_complete_refs
> > >> >                    else
> > >> >                            __gitcomp_nl "$(__git stash list \
> > >> > -                                          | sed -n -e 's/:.*//p')"
> > >> > +                                          | sed -n -e 's/:.*//p' \
> > >> > +                                          | sed 's/@/\\@/')"
> >
> > The first and the original one -n -e "s/:.*//p" wants to show no
> > lines by default, unless it finds a colon on the line and strip it
> > and everything that follows.  You then further want to tweak that
> > and prefix a backslash before the first '@' you find.  So perhaps
> > something along the lines of...
> >
> >         sed -n -e '/:/{
> >                 s/:.*//
> >                 s/@/\\@/
> >                 p
> >         }'
> >
> > ... which is "show no lines by default, but on a line with a colon,
> > strip the colon and everything that follows, and then prefix the
> > first '@' on the line, if exists, with backslash, and show the result".
> >
> > Having said that, I am very skeptical to any "solution" that has to
> > show "stash\@{" to end-users.  And with the patch, newer versions of
> > bash does seem to show the stupid "all stash entries, numbered",
> > i.e.
> >
> >     $ git stash show stas<TAB>
> >     -->
> >     $ git stash show stash\@{
> >
> >     $ git stash show stash\@{<TAB>
> >     stash\@{0}   stash\@{1}
> >
> > I wonder what unpleasant end-user experience I may have to endure if
> > I had dozens of stash entries.  The list shows only the numbers, so
> > it does not help me decide which number to type at all.
> >
> > A solution to allow older versions of bash to complete
> >
> >     $ git stash show stas<TAB>
> >     -->
> >     $ git stash show stash@{
> >
> >     $ git stash show stash@{<TAB>
> >     -->
> >     $ git stash show stash@{
> >
> > would be very much welcomed, though.
> >
> > Thanks.
> >
> >
> >
>