Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OleHolmNielsen wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a2d924
Added no-arg options -b and -B to bash_completion.d/clush
OleHolmNielsen Dec 23, 2025
fbd3957
Try joining two lines
OleHolmNielsen Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bash_completion.d/clush
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ _clush()
-w|-g|--group) target_set=1; skip=any;;
# no-arg options
--version|-h|--help|-n|--nostdin|-a|--all|-q|--quiet|\
-v|--verbose|-d|--debug) ;;
-b|-B|-v|--verbose|-d|--debug) ;;
Copy link
Collaborator

@martinetd martinetd Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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...)

Copy link
Contributor Author

@OleHolmNielsen OleHolmNielsen Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@martinetd martinetd Dec 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

@OleHolmNielsen OleHolmNielsen Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@martinetd martinetd Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.. :))

Copy link
Contributor Author

@OleHolmNielsen OleHolmNielsen Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# get source separately...
--groupsource=*) groupsource="${word#*=}";;
-s|--groupsource) skip=groupsource;;
Expand Down
Loading