Re: [PR] Add zsh support to completion script [maven-mvnd]

["gnodet (via GitHub)" <gi...@apache.org>]

gnodet commented on code in PR #1559:
URL: https://github.com/apache/maven-mvnd/pull/1559#discussion_r3741187341


##########
dist/src/main/distro/bin/mvnd-bash-completion.bash:
##########
@@ -17,10 +17,26 @@
 # Adapted from https://github.com/juven/maven-bash-completion/blob/master/bash_completion.bash by Juven Xu and others
 # under Apache License Version 2.0
 
+# Detect shell type and set up zsh compatibility if needed
+if [ -n "$ZSH_VERSION" ]; then
+    __MVND_SHELL="zsh"
+    # Load bashcompinit for bash-style completions in zsh
+    autoload -Uz bashcompinit 2>/dev/null && bashcompinit
+    # Define COMP_WORDBREAKS if not set (bash sets this automatically)

Review Comment:
   **Suggestion (medium):** The zsh documentation states that `bashcompinit` must be called after `compinit` has been loaded. This script calls `autoload -Uz bashcompinit && bashcompinit` without first ensuring `compinit` is initialized. Most zsh users will already have `compinit` loaded via their framework (oh-my-zsh, prezto, etc.) or `.zshrc`, but users with a minimal zsh configuration will get silent failure — the `2>/dev/null` suppresses the error and completions simply won't register.
   
   Consider adding `compinit` initialization before `bashcompinit`:
   ```suggestion
       __MVND_SHELL="zsh"
       # Load zsh completion system if not already initialized
       autoload -Uz compinit 2>/dev/null && compinit -u 2>/dev/null
       # Load bashcompinit for bash-style completions in zsh
       autoload -Uz bashcompinit 2>/dev/null && bashcompinit
   ```
   
   Alternatively, document the `compinit` prerequisite in the README.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org