Improved lmod_bash_completions #157
Conversation
pneerincx
commented
Aug 31, 2016
pneerincx
commented
- Added support for ${LMOD_REDIRECT}
- Fixed bug where "ml rm " would list all available modules as opposed to loaded ones that can be removed.
- Indentation used a mix of tabs and spaces: standardized for now on tabs for indentation.
… bug where ml rm <TAB> would list all available module as opposed to loaded ones that can be removed.
Regular pull from blessed master.
| COMPREPLY=( $(IFS=: compgen -W "${MODULEPATH}" -- "${cur}") ) | ||
| ;; | ||
| use|*-a*) | ||
| _filedir -d || return 0 |
There was a problem hiding this comment.
@pneerincx: We recently noticed that tab completion on use requires _filedir, which is not available by default in bash (it's not a builtin like compgen is), but is provided by a separate package (e.g. bash-completion on SL6)
Wouldn't it be better to check whether _filedir is defined rather than using it blindly?
I think that's what the return 0 is about, but it doesn't do what it's intended to do.
Here's what I get on a system that doesn't have _filedir with ml use /user<tab>:
$ ml use /user-bash: _filedir: command not found
…n't available on a system (issue TACC#86).
| COMPREPLY[0]="${dir}/" | ||
| compopt -o nospace | ||
| fi | ||
| fi |
There was a problem hiding this comment.
@pneerincx wouldn't it be cleaner to define a dedicated function for this, rather than copy-pasting it?
|
Thanks very much for the patch. Coming up with a way to handle sites that do not have _filedir is a great fix (repairing #86)! I prefer the indentation that was already in the original so I have reverted it. I do agree that mixing tabs and spaces is a bad idea so I have marked the file to only use spaces for indentation. I also took @boegel suggestion to combine the duplicated code into a single function called _module_filedir() This is fix is marked as Lmod 6.5.5. Please test this new version when you get a chance. |
