git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround
@ 2013-06-09 23:59 SZEDER Gábor
  2013-06-10  0:09 ` Jonathan Nieder
  2013-06-10  1:58 ` Eric Sunshine
  0 siblings, 2 replies; 5+ messages in thread
From: SZEDER Gábor @ 2013-06-09 23:59 UTC (permalink / raw)
  To: Junio C Hamano, git; +Cc: SZEDER Gábor

Fedore 9 shipped the gvfs package with a buggy Bash completion script:
it removed the ':' character from COMP_WORDBREAKS, thereby breaking
certain features of git's completion script.  We worked this around in
db8a9ff0 (bash completion: Resolve git show ref:path<tab> losing ref:
portion, 2008-07-15).

The bug was fixed in Fedora 10 and Fedora 9 reached its EOL on
2009-07-10, almost four years ago.  It's about time to remove our
workaround.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 91234d47..2f78dcfa 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -24,11 +24,6 @@
 #    3) Consider changing your PS1 to also show the current branch,
 #       see git-prompt.sh for details.
 
-case "$COMP_WORDBREAKS" in
-*:*) : great ;;
-*)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
-esac
-
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
 __gitdir ()
@@ -458,11 +453,6 @@ __git_complete_revlist_file ()
 			;;
 		esac
 
-		case "$COMP_WORDBREAKS" in
-		*:*) : great ;;
-		*)   pfx="$ref:$pfx" ;;
-		esac
-
 		__gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
 				| sed '/^100... blob /{
 				           s,^.*	,,
@@ -560,10 +550,6 @@ __git_complete_remote_or_refspec ()
 	[ "$remote" = "." ] && remote=
 	case "$cur_" in
 	*:*)
-		case "$COMP_WORDBREAKS" in
-		*:*) : great ;;
-		*)   pfx="${cur_%%:*}:" ;;
-		esac
 		cur_="${cur_#*:}"
 		lhs=0
 		;;
-- 
1.8.0.269.g97125f4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround
  2013-06-09 23:59 [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround SZEDER Gábor
@ 2013-06-10  0:09 ` Jonathan Nieder
  2013-06-10  9:14   ` SZEDER Gábor
  2013-06-10  1:58 ` Eric Sunshine
  1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Nieder @ 2013-06-10  0:09 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, git

SZEDER Gábor wrote:

> Fedore 9 shipped the gvfs package with a buggy Bash completion script:
> it removed the ':' character from COMP_WORDBREAKS, thereby breaking
> certain features of git's completion script.  We worked this around in
> db8a9ff0 (bash completion: Resolve git show ref:path<tab> losing ref:
> portion, 2008-07-15).
>
> The bug was fixed in Fedora 10 and Fedora 9 reached its EOL on
> 2009-07-10, almost four years ago.  It's about time to remove our
> workaround.

Nice!  I had wondered what that workaround was about but never
ended up digging into it.

Searching for COMP_WORDBREAKS in /etc/bash_completion.d/* finds
similar breakage (removal of '=' and '@') in the npm completion
script, but nothing involving colon.  So this looks like a good
change.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround
  2013-06-09 23:59 [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround SZEDER Gábor
  2013-06-10  0:09 ` Jonathan Nieder
@ 2013-06-10  1:58 ` Eric Sunshine
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Sunshine @ 2013-06-10  1:58 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, Git List

On Sun, Jun 9, 2013 at 7:59 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> Fedore 9 shipped the gvfs package with a buggy Bash completion script:

s/Fedore/Fedora/

> it removed the ':' character from COMP_WORDBREAKS, thereby breaking
> certain features of git's completion script.  We worked this around in
> db8a9ff0 (bash completion: Resolve git show ref:path<tab> losing ref:
> portion, 2008-07-15).
>
> The bug was fixed in Fedora 10 and Fedora 9 reached its EOL on
> 2009-07-10, almost four years ago.  It's about time to remove our
> workaround.
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround
  2013-06-10  0:09 ` Jonathan Nieder
@ 2013-06-10  9:14   ` SZEDER Gábor
  2013-06-10 20:48     ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: SZEDER Gábor @ 2013-06-10  9:14 UTC (permalink / raw)
  To: Jonathan Nieder, Junio C Hamano; +Cc: git


First things first: Junio, please don't pick up this patch.

On Sun, Jun 09, 2013 at 05:09:54PM -0700, Jonathan Nieder wrote:
> SZEDER Gábor wrote:
> 
> > Fedore 9 shipped the gvfs package with a buggy Bash completion script:
> > it removed the ':' character from COMP_WORDBREAKS, thereby breaking
> > certain features of git's completion script.  We worked this around in
> > db8a9ff0 (bash completion: Resolve git show ref:path<tab> losing ref:
> > portion, 2008-07-15).
> >
> > The bug was fixed in Fedora 10 and Fedora 9 reached its EOL on
> > 2009-07-10, almost four years ago.  It's about time to remove our
> > workaround.
> 
> Nice!  I had wondered what that workaround was about but never
> ended up digging into it.

Meh, not nice at all.

> Searching for COMP_WORDBREAKS in /etc/bash_completion.d/* finds
> similar breakage (removal of '=' and '@') in the npm completion
> script, but nothing involving colon.  So this looks like a good
> change.

And apparently I have a completion script called axi-cache (from
package apt-xapian-index) which removes ':' [1].  However, it doesn't
remove the ':' upon loading the script but the removal is done in the
completion function, i.e. it takes effect only when the user actually
attempts to complete its options.  I never use axi-cache, whatever the
hell it might be, so I didn't notice.

Unfortunately, with my above patch applied I get this in a new shell:

$ git show master:q<TAB>
$ git show master:quote.<TAB>
master:quote.c   master:quote.h   

$ axi-cache <TAB>
again     depends   help      madison   more      policy    rdepends
search    show      showpkg   showsrc   

$ git show master:q<TAB>
$ git show quote.<TAB>
quote.c  quote.h  quote.o

Not good.

I don't have the npm package, but manually removing '=' from
COMP_WORDBREAKS leads to similar breakage with e.g. 'git log
--pretty='.

Neither this axi-cache not npm completion script comes from the
bash-completion project.  Apparently some developers providing
completion scripts for their projects lack the necessary expertise to
consider the effects their script might have on other completion
scripts.  Perhaps distributions should adopt a policy not to allow
completion scripts messing with COMP_WORDBREAKS, dunno.

Anyway.  Although the completion script in Fedora's gvfs package might
be fixed, there are other completion scripts making the same mistake,
so I'm afraid we should keep the workaround and drop this patch.
Moreover, we should even consider extending our workaround by adding
back '=' to COMP_WORDBREAKS, too.  Oh, well.


[1] - This might not be accurate nowadays, as my system is a bit
oldish...


Best,
Gábor

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround
  2013-06-10  9:14   ` SZEDER Gábor
@ 2013-06-10 20:48     ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2013-06-10 20:48 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Jonathan Nieder, git

SZEDER Gábor <szeder@ira.uka.de> writes:

> Anyway.  Although the completion script in Fedora's gvfs package might
> be fixed, there are other completion scripts making the same mistake,
> so I'm afraid we should keep the workaround and drop this patch.
> Moreover, we should even consider extending our workaround by adding
> back '=' to COMP_WORDBREAKS, too.  Oh, well.

Thanks.

Perhaps the summary of your problem analysis deserves to be added as
a in-code comment to make sure others won't later try to remove it
only by checking how old Fedora 9 is?

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-06-10 20:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09 23:59 [PATCH] completion: remove ancient "ensure colon in COMP_WORDBREAKS" workaround SZEDER Gábor
2013-06-10  0:09 ` Jonathan Nieder
2013-06-10  9:14   ` SZEDER Gábor
2013-06-10 20:48     ` Junio C Hamano
2013-06-10  1:58 ` Eric Sunshine

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).