On Thu, Nov 29, 2012 at 09:33:53AM -0800, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > Adam Tkac <atkac@xxxxxxxxxx> writes: > > > >> Subject: Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion > > > > The code does not seem to do anything special if it is not aliased, > > though, so "If ..." part does not sound correct; perhaps you meant > > "just in case egrep is aliased to something totally wacky" or > > something? > > > > The script seems to use commands other than 'egrep' that too can be > > aliased to do whatever unexpected things. How does this patch get > > away without backslashing them all, like > > > > \echo ... > > \sed ... > > \test ... > > \: comment ... > > \git args ... > > > > and still fix problems for users? Can't the same solution you would > > give to users who alias one of the above to do something undesirable > > be applied to those who alias egrep? > > > > Puzzled... > > Sorry for having been more snarky than necessary (blame it to lack > of caffeine). What I was trying to get at were: > > * I have this suspicion that this patch exists only because you saw > somebody who aliases egrep to something unexpected by the use of > it in this script, and egrep *happened* to be the only such > "unreasonable" alias. The reporter may not have aliased echo or > sed away, or the aliases to these command *happened* to produce > "acceptable" output (even though it might have been slightly > different from unaliased one, the difference *happened* not to > matter for the purpose of this script). > > * To the person who observes the same aliasing breakage due to his > aliasing sed to something else, you would solve his problem by > telling him "don't do that, then". If that is the solution, why > wouldn't it work for egrep? > > * The next person who aliased other commands this script uses in > such a way that the behaviour of the alias differs sufficiently > from the unaliased version, you will have to patch the file > again, with the same backslashing. This patch is not a solution, > but a band-aid that only works for a particular case you > *happened* to have seen. > > * A complete solution that follows the direction this patch > suggests would involve backslashing *all* commands that can > potentially aliased away. Is that really the direction we would > want to go in (answer: I doubt it)? Is that the only approach to > solve this aliasing issue (answer: I don't know, but we should > try to pursue it before applying a band-aid that is not a > solution)? > > Is there a way to tell bash "do not alias-expand from here up to > there"? Perhaps "shopt -u expand_aliases" upon entry and restore > its original value when we exit, or something? > > IOW, something along this line? This won't work, unfortunately, because shopt settings aren't inherited by subshell (and for example egrep is called in subshell). I discussed this issue with colleagues and we found basically two "fixes": 1. Tell people "do not use aliases which breaks completion script" 2. Prefix all commands with "command", i.e. `command egrep` etc. In my opinion "2." is better long time solution, what do you think? Regards, Adam > > contrib/completion/git-completion.bash | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git i/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash > index 0b77eb1..193f53c 100644 > --- i/contrib/completion/git-completion.bash > +++ w/contrib/completion/git-completion.bash > @@ -23,6 +23,14 @@ > # 3) Consider changing your PS1 to also show the current branch, > # see git-prompt.sh for details. > > +if shopt -q expand_aliases > +then > + _git__aliases_were_enabled=yes > +else > + _git__aliases_were_enabled= > +fi > +shopt -u expand_aliases > + > case "$COMP_WORDBREAKS" in > *:*) : great ;; > *) COMP_WORDBREAKS="$COMP_WORDBREAKS:" > @@ -2504,3 +2512,8 @@ __git_complete gitk __gitk_main > if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then > __git_complete git.exe __git_main > fi > + > +if test -n "$_git__aliases_were_enabled" > +then > + shopt -s expand_aliases > +fi > > -- Adam Tkac, Red Hat, Inc. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html