TAB completion in Bash adds extraneous space for modules in subdirectories

[mgsternberg]

Describe the bug

TAB completion in Bash adds extraneous space for module names containing /, i.e., having subdirectories.

To Reproduce

• Assuming a system offers:

$ module -t avail intel
…
intel/19/19.1.3
intel/2021/2021.4.0
…

• On the Bash prompt, type:
  $ module load intel
• Press TAB key.
• Completion inserts / (slash and space), and places the cursor (indicated as _) afterwards:
  module load intel/ _

Expected behavior

• Insert / (slash only):
  $ module load intel/_

Error and debugging information

Sorry, not applicable.

Modules version and configuration

$ module --version
Modules Release 5.2.0 (2022-11-08)

$ echo $BASH_VERSION 
4.2.46(2)-release

$ module config --dump-state | grep -i compl
## (not applicable)

Cause

A sh(1) syntax bug in file /usr/share/bash-completion/completions/module, line 7.

Discussion

The original code has:

     local setnospace=1
     …
     if [ ! $setnospace ] && [ "${val: -1:1}" = '/' ]; then
     …
             setnospace=0

Thus, the sh construct [ ! $setnospace ] expands to either [ ! 0 ] or [ ! 1 ],

The trouble is that both of these evaluate to false because the ! operator on a string for POSIX shells (not pure Bourne) tests for emptiness, not numerical value:

$ [ ! 0 ] || echo false
false
$ [ ! 1 ] || echo false
false

# But:
$ [ ! '' ] || echo false
$

# Also consider the reverse:
$ [ 0 ] && echo true
true
$ [ 1 ] && echo true
true
$ [ '' ] && echo true
$

Suggested patch

Retaining the test logic as written (not using [[ … ]]), compare $setnospace explicitly against 1 (happens to be a string value), and drop the negation to simplify comprehension in light of the option name nospace already containing a negation.

--- /usr/share/bash-completion/completions/module.orig	2023-05-09 14:54:35.000000000 -0500
+++ /usr/share/bash-completion/completions/module	2023-05-09 16:28:29.410573374 -0500
@@ -7,7 +7,7 @@
     local setnospace=1
     # do not append space to word completed if it is a directory (ends with /)
     for val in $(compgen -W "$1" -- "$2"); do
-        if [ ! $setnospace ] && [ "${val: -1:1}" = '/' ]; then
+        if [ $setnospace = 1 ] && [ "${val: -1:1}" = '/' ]; then
             # Bash >=4.0 is required for compopt
             type compopt &>/dev/null && compopt -o nospace
             setnospace=0

[mgsternberg]

Correction: My suggested patch was for file names in the as-installed-package on EL7. In the source tree, the patch needs to be applied to file init/bash_completion.in, of course:

--- modules-5.2.0/init/bash_completion.in.orig       2022-11-07 22:08:06.000000000 -0600
+++ modules-5.2.0/init/bash_completion.in       2023-05-09 17:41:42.000000000 -0500
@@ -7,7 +7,7 @@
     local setnospace=1
     # do not append space to word completed if it is a directory (ends with /)
     for val in $(compgen -W "$1" -- "$2"); do
-        if [ ! $setnospace ] && [ "${val: -1:1}" = '/' ]; then
+        if [ $setnospace = 1 ] && [ "${val: -1:1}" = '/' ]; then
             # Bash >=4.0 is required for compopt
             type compopt &>/dev/null && compopt -o nospace
             setnospace=0

[xdelaruelle]

Many thanks @mgsternberg for this detailed report and proposed solution.

Code issue was there since a long time but it was kind of silent until we tried to fix SC2166 ShellCheck warning in 6530983.

I was notified on this bug few weeks ago and reported it as #490. I fixed it with 5e89196 with an approach very close to what you proposed. The fix will be available in next release (v5.3.0).

Best regards,
Xavier

[mgsternberg]

Excellent, I'm glad you addressed it. Thank you!

I'm sorry for the duplicate — I had searched for completion prior to posting, but the is:open tag masked #490.