Shell completion script breaks bash's environment variable assignment filesystem path completion #4530

Closed
aadamowski opened this Issue Jan 22, 2014 · 12 comments

Comments

Projects
None yet
8 participants

The shell completion script lib/utils/completion.sh has an unfortunate effect on Bash that it breaks standard TAB completion for setting environment variable values.

As reported on ServerFault, without npm shell completion the correct expected behaviour is as follows:

  1. the user types:
export SOME_VARIABLE_NAME=/partial/filesystem/path/to/someth
  1. the user presses TAB

  2. the command line gets completed to:

export SOME_VARIABLE_NAME=/partial/filesystem/path/to/something_existing

With npm shell completion active, the result is broken - the environment variable name gets removed:

  1. the command line gets completed to:
export /partial/filesystem/path/to/something_existing

As reported here, the actual source of the problem is that the completion script removes the '=' character from COMP_WORDBREAKS:

COMP_WORDBREAKS=${COMP_WORDBREAKS/=/}
Contributor

jasonkarns commented Mar 23, 2015

I ran into this bug recently. Without the equal sign, git completion breaks for any option containing an equal sign.

git log --date=<TAB>, git commit --reuse-message=<TAB>, git branch --set-upstream-to=<TAB> are all broken because of npm's modification of COMP_WORDBREAKS.

This might be solved using the solution proposed in this stackoverflow thread
I'm a complete bash noob, I can't make sense of the syntax in the accepted answer, so I won't be able to propose a patch : (

whitty commented May 7, 2015

I had a go at rewriting the completion script and it seems to work for me - see 7e4d15f

I find firstly that completions with = and @ are no longer broken outside of npm, and npm completion seems to work fairly well.

That being said I'm neither a bash_completion export, nor an npm user so I don't know if I've broken anything.

Contributor

jasonkarns commented Jun 25, 2015

Clearly the original author of this completion script had a use case in mind for ignoring @ and = in the word completions. Do we have an example command that exercises this use case so that we can test @whitty's implementation?

Contributor

jasonkarns commented Jun 25, 2015

Also, this change seems to be working just fine for me (ie, not breaking git's tab completion)

GNU bash, version 4.3.33(1)-release (x86_64-apple-darwin13.4.0)

Hi! Took me quite a while to discover it was /etc/bash_complete.d/npm that broke my filename completion after an equal sign, and finally found this issue. ;-)

For completeness sake, this bug was also reported against the Debian's npm package, which uses same bash completion script:

@othiym23 othiym23 added a commit that referenced this issue Aug 7, 2015

@othiym23 Greg Whiteley + othiym23 completion: don't break global COMP_WORDBREAKS
As described in #4530, #5820 modification of global variable
COMP_WORDBREAKS causes global breakage of completion:

```
dd if=/dev/ze<tab>
->
dd /dev/zero
```

Fixes: #4530

PR-URL: #8892
d7271b8
Contributor

othiym23 commented Aug 7, 2015

There is a fix landed to the completion script in d7271b8 – the lib/utils/completion.sh included in the tree at that version (which will be part of the next npm@2 release, coming out later tonight as npm@next) fixes this issue. I'm not sure we can say that resolves this issue, though, because the patched script will need to get pushed to all the downstream distributions bundling it. @anthonyfok, could you do us a favor and update that Debian bug with this information?

I'll leave this open for a little while as a reference point for people discussing this with distros. Thanks for your patience, everybody!

Contributor

jasonkarns commented Aug 7, 2015

Do we need to open a PR to get this into homebrew?

@iarna iarna added a commit that referenced this issue Aug 7, 2015

@iarna Greg Whiteley + iarna completion: don't break global COMP_WORDBREAKS
As described in #4530, #5820 modification of global variable
COMP_WORDBREAKS causes global breakage of completion:

```
dd if=/dev/ze<tab>
->
dd /dev/zero
```

Fixes: #4530

PR-URL: #8892
2525b95

@iarna iarna added a commit that referenced this issue Aug 7, 2015

@iarna Greg Whiteley + iarna completion: don't break global COMP_WORDBREAKS
As described in #4530, #5820 modification of global variable
COMP_WORDBREAKS causes global breakage of completion:

```
dd if=/dev/ze<tab>
->
dd /dev/zero
```

Fixes: #4530

PR-URL: #8892
04ec77b
Contributor

jasonkarns commented Aug 14, 2015

Yay! 2.13.5 is now @latest !!!

Thanks @whitty !

@anthonyfok, could you do us a favor and update that Debian bug with this information?

Hi @othiym23, sorry for the late reply. I have just sent an update to Debian Bug #711810 (https://bugs.debian.org/711810) accordingly. Hope we will see npm@2.13.5 in Debian soon! :-)
(See Debian Bug #794890: npm: new upstream version.)

busches commented Sep 26, 2016

Can this issue be closed since the problem itself is resolved? Debian is still including npm@1 on their repositories, but that should be filed against Debian, not npm. https://packages.debian.org/search?searchon=names&keywords=npm

cc\ @KenanY

KenanY closed this Sep 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment