Add no-arg options -b and -B to bash_completion.d/clush

[OleHolmNielsen]

We mostly user the clush -b <args> option to aggregate output from our cluster nodes or other groups of nodes.
We also use the Bash TAB completions added by PR #563 and part of the 1.9.3 release which we install as an EL8 RPM from EPEL.

However, combining clush -b -g<TAB><TAB> doesn't seem to work, whereas clush -g<TAB><TAB> -b works just fine.

I think this issue is fixed by adding no-arg options -b and -B to bash_completion.d/clush.

Thanks,
Ole

[OleHolmNielsen]

I don't understand why the tests fail on this simple addition. Can you assist me?

[martinetd]

However, combining clush -b -g doesn't seem to work, whereas clush -g -b works just fine.

hmm, I can't reproduce -- in practice that -b is not listed in no-arg options will eat the next word and miss e.g. -s setting, so your patch is correct, but it shouldn't be needed for clush -b -g<TAB> because $prev/$cur will still be -g/whatever argument

You said el8 so I guess I ought to try on an el8 bash?
Well, it is a proper fix anyway, so I guess if it works for you it's good for me as well.

[martinetd]

(nitpick) if we're going to add this might as well add other no-arg options from help:
-G|--groupbase|-L|-N|-P|--progress|-b|--dshbak|-B|-r|--regroup|-S|--maxrc|--diff|-p

(other standalone options such as --color=* are caught in the --*=* later; I'm honestly not sure why I stopped at debug...)

[OleHolmNielsen]

Hi Dominique,
Thanks a lot for considering this patch!
I tested it successfully on AlmaLinux 8.10 (Bash 4.4.20) and RockyLinux 9.7 (Bash 5.1.8) by manually adding -b|-B to /usr/share/bash-completion/completions/clush as installed by the RPM:

clustershell-1.9.3-1.el9.noarch

I have no idea why the Github checks are failing my simple patch?
Actually, I mostly use the -b option together with -g <group> like in this example:

clush -bg alma9 uname -r

Would it be possible to have also -bg <group> as a permitted command completion? That would be convenient, although only optional and not necessary.
Thanks a lot, and Happy New Year,
Ole

[martinetd]

I tested it successfully on AlmaLinux 8.10 (Bash 4.4.20) and RockyLinux 9.7 (Bash 5.1.8) by manually adding [..]

I mean yes clush -b -g ... works with this patch, but for me it also works without, so I was a bit lost :)
(FWIW you can try this without installing by just sourcing the completion script, bash will not overwrite anything you sourced manually)

clush -bg alma9 uname -r

Hmm, right, -b -g works, -bg doesn't right now -- the code does handle -ggroup without space, but there's nothing handling any non-argument letters earlier...
This makes the script even harder to understand, but this seems to mostly work from quick testing:

diff --git a/bash_completion.d/clush b/bash_completion.d/clush
index a465f27543ed..3500afb73635 100644
--- a/bash_completion.d/clush
+++ b/bash_completion.d/clush
@@ -36,7 +36,7 @@ _clush()
 	local cur prev words cword split
 	local i word options="" compopts="" skip=argv0 groupsource="" cleangroup=""
 	local mode=command target_set=""
-	local shortopt=""
+	local shortopt="" full_cur
 
 	_init_completion -s -n : || return
 
@@ -60,14 +60,21 @@ _clush()
 			_clush_command_or_file
 			return
 			;;
-		-c|--copy|--rcopy) mode=copy;;
-		-w|-g|--group) target_set=1; skip=any;;
+		-c*|-[!-]*c|--copy|--rcopy) mode=copy;;
+		-w|-[!-]*w|-g|-[!-]*g|--group) target_set=1; skip=any;;
+		# destination with arg added without space
+		-[wg][!\ ]*|-[!-]*[wg][!\ ]*) target_set=1;;
 		# no-arg options
 		--version|-h|--help|-n|--nostdin|-a|--all|-q|--quiet|\
-		-v|--verbose|-d|--debug) ;;
+		-v|--verbose|-d|--debug|-G|--groupbase|-L|-N|-P|--progress|\
+		-b|--dshbak|-B|-r|--regroup|-S|--maxrc|--diff|-p) ;;
+		# exclude... without and with arg
+		-x|-[!-]*x|-X|-[!-]X) skip=any;;
+		-x[!\ ]*|-[!-]*x[!\ ]*|-X[!\ ]*|-[!-]X[!\ ]*) ;;
 		# get source separately...
 		--groupsource=*) groupsource="${word#*=}";;
-		-s|--groupsource) skip=groupsource;;
+		-s|-[!-]*s|--groupsource) skip=groupsource;;
+		-s[!\ ]*|-[!-]s[!\ ]*) groupsource=${word#-*s};;
 		# assume all the rest as options...
 		# options with = included in word
 		--*=*) ;;
@@ -82,13 +89,21 @@ _clush()
 	done
 
 	# split short opts without space...
+	full_cur="$cur"
 	case "$cur" in
-	-[a-z]*)
-		shortopt="${cur:0:2}"
+	-[!-]*[wgsxX]*)
+		local suffix=${cur#-*[wgsxX]}
+		shortopt=${cur%"$suffix"}
 		prev="$shortopt"
-		cur="${cur:2}"
+		cur="$suffix"
 		;;
 	esac
+	case "$prev" in
+	-[!-]*[wgsxX][!\ ]*) ;; # already has an arg, skip
+	-[!-]*[wgsxX])
+		# keep last letter to simplify below case
+		prev="-${prev:${#prev}-1:1}";;
+	esac
 
 	case "$prev" in
 	-w|-x|-g|--group|-X)
@@ -153,5 +168,5 @@ _clush()
 			| sed -e 's/[^:]$/& /' -e "s/^/$shortopt/")
 	# remove the prefix from COMPREPLY if $cur contains colons and
 	# COMP_WORDBREAKS splits on colons...
-	__ltrim_colon_completions "$cur"
+	__ltrim_colon_completions "$full_cur"
 } && complete -o nospace -F _clush "${BASH_SOURCE##*/}"

I'm.. honestly not convinced we want to go that far (at this point we're really reimplementing the parsing in bash...), but I don't think it'd break anything, and part of the above (with full_cur) is actually a fix needed for -w@source:... anyway, so I guess if you're fine with it I can clean this up in a few commits and push to your branch (if I can) or open a new PR...

I have no idea why the Github checks are failing my simple patch?

yeah, I had a quick look and the parent commit passes so I don't know either, this is unrelated so just ignore and @thiell will get around to it eventually :)

[OleHolmNielsen]

Thanks, I removed my patch manually and did:

# clush -b -g alm[TAB][TAB]
Usage: clush [options] command

clush: error: No node to run on.

When I reintroduce my patch it works as expected again:

[root@servauth5 ~]# clush -b -g alma[TAB][TAB]
alma    alma8   alma9
[root@servauth5 ~]# clush -b -g alma9 uname -r
---------------
camd-runner2,demon[2-3],intra7,mirror4,rsnap[2-3],web[6-7] (9)
---------------
5.14.0-611.16.1.el9_7.x86_64

This is RockyLinux 9.7 (Bash 5.1.8).

Let's ignore my -bg suggestion since we don't want to introduce complexity.
Thanks,
Ole

[martinetd]

I really don't understand why it wouldn't work as is -- what doesn't work is that clush -b -g alma9 command won't properly complete the command, but the alma9 bit should complete (and this works on bash-5.3.0-2.fc43.x86_64 / bash-completion-2.16-2.fc43.noarch)...

Anyway, as said in the first place, it is the correct thing to do -- please just add all of -G|--groupbase|-L|-N|-P|--progress|-b|--dshbak|-B|-r|--regroup|-S|--maxrc|--diff|-p and not just -b|-B -- the diff I wrote above has it if you have trouble with line continuations.

As far as I'm concerned this PR can get in with just that fixed (and we can worry about complexity and -bg next year.. :))

[OleHolmNielsen]

Thanks a lot! FC43 is way ahead of Rocky/Alma/RHEL 9 in many ways, so maybe this is why it works for you?
Or maybe my problem is that we have upgraded ClusterShell during many releases (always from the EPEL repo) so that some old configs are causing problems - I don't know.
So I'll add your suggested configs, and look forward to a future release of your excellent tool. It's much appreciated!
Ole