Skip to content

Add zsh support to completion script - #1559

Open
gnodet wants to merge 1 commit into
masterfrom
unified-bash-zsh-completion
Open

Add zsh support to completion script#1559
gnodet wants to merge 1 commit into
masterfrom
unified-bash-zsh-completion

Conversation

@gnodet

@gnodet gnodet commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add zsh support to the existing mvnd-bash-completion.bash script (auto-detects shell type)
  • Accept zsh as valid --completion argument in Completion.java
  • Update README.adoc with zsh instructions

Supersedes #1514 by consolidating the zsh compatibility into the single existing script rather than adding a separate file. This avoids duplication and maintenance burden.

Approach

The script auto-detects bash vs zsh and adapts three shell-specific behaviors:

  • function_exists: uses typeset -f (zsh) vs declare -F (bash)
  • Plugin variable discovery: uses typeset + (zsh) vs compgen -v (bash)
  • Indirect variable expansion: uses ${(P)var} (zsh) vs ${!var} (bash)

For zsh, bashcompinit is loaded to provide bash-style completion builtins (complete, compgen, COMPREPLY). No global shell options (setopt) are modified.

Credit

Based on the approach from #1514 by @janweinschenker.

Test plan

  • Full build passes
  • Completion template regeneration via CompletionGeneratorTest succeeds
  • Source the script in bash: works as before
  • Source the script in zsh: completions work via bashcompinit layer

🤖 Generated with Claude Code

Make the existing bash completion script work in both bash and zsh
by auto-detecting the shell and adapting behavior accordingly:

- Add shell detection via $ZSH_VERSION / $BASH_VERSION
- Load bashcompinit in zsh for bash-style completion compatibility
- Use typeset instead of declare/compgen for zsh
- Use ${(P)var} instead of ${!var} for indirect expansion in zsh
- Accept 'zsh' as valid --completion argument in Completion.java
- Update README.adoc with zsh instructions

Based on the approach from PR #1514 by janweinschenker, consolidated
into the single existing script to avoid duplication.

Co-Authored-By: Jan Weinschenker <janweinschenker@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@janweinschenker

janweinschenker commented Mar 27, 2026

Copy link
Copy Markdown

What I tested on zsh:

$ mvnd [TAB]
# resulted in:

$ mvnd 
clean                    generate-sources         install                  post-integration-test    pre-site                 process-sources          site                     validate
compile                  generate-test-resources  integration-test         post-site                prepare-package          process-test-classes     site-deploy              verify
deploy                   generate-test-sources    package                  pre-clean                process-classes          process-test-resources   test                     
generate-resources       initialize               post-clean               pre-integration-test     process-resources        process-test-sources     test-compile  
$ mvnd cl [TAB]
# resulted in:
$ mvnd clean
$ mvnd clean pa [TAB]
# resulted in:
$ mvnd clean package
$ mvnd -pl m
# correctly gave me a list with all sub-modules
$ mvn -P [TAB][TAB]
# resulted in:
-1    -B    -C    -D    -N    -P    -T    -U    -V    -X    -am   -amd  -c    -cpu  -e    -emp  -ep   -f    -fae  -ff   -fn   -gs   -h    -l    -npr  -npu  -nsu  -o    -pl   -q    -rf   -s    -t    -up   -v

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Clean, well-structured PR that adds zsh support to the existing bash completion script. The three shell-specific adaptations (function detection via typeset -f, variable discovery via typeset +, indirect expansion via ${(P)var}) are correctly implemented using standard zsh idioms, and the bashcompinit approach is simpler than the superseded PR #1514.

Both copies of the completion script (dist and test template) are properly synchronized.

One minor suggestion: consider adding a completionZsh() test in CompletionNativeIT.java to exercise the new --completion zsh code path. Since both shells return the same script, the assertions can be identical to the bash test, but it would guard against regressions in the argument validation.

LGTM.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AI-assisted review — looks good ✅

Clean, well-scoped PR that adds zsh compatibility to the existing bash completion script via shell auto-detection. The three zsh-specific adaptations are correctly implemented using standard idioms, and the consolidation approach (single file instead of a duplicate) is superior to the superseded PR #1514.

Good design choices: keeping a single auto-detecting script avoids duplication, not calling compinit unconditionally avoids shell startup slowdown, and the __MVND_SHELL double-underscore prefix is a good convention to avoid variable collisions. Both copies of the completion script (dist and test template) are correctly kept in sync.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants