Skip to content

Conversation

@maxlandon
Copy link
Contributor

@maxlandon maxlandon commented Aug 1, 2025

This pull request introduces a significant enhancement to the Zsh completion engine, completely overhauling how completion suffixes are managed. The new system delegates suffix handling to Zsh's native completion facilities, providing a more robust, intuitive, and consistent user experience for suffixes like the default trailing space, directory slashes (/), and other separators (,, :, etc.). This PR addresses the topic raised in issue #666.

Key Changes

  • Suffix-Aware Value Processing (internal/shell/zsh/action.go):
    • The Go code now intelligently splits completion values from their suffixes. For example, a value like mydir/ is separated into the core value mydr and the suffix /.
    • The default trailing space is no longer manually appended; it is now treated as a distinct suffix type to be handled by the shell.
    • This structured information (value, display, suffix) is passed to the Zsh script, enabling fine-grained control over completion behavior.

Zsh Snippet Deep Dive

The core of this enhancement lies in the complete rewrite of the completion processing loop within the Zsh snippet (internal/shell/zsh/snippet.go). The script is now more efficient and uses idiomatic Zsh features.

1. Efficient Data Parsing

The script now uses direct array initialization instead of multiple read commands, which is cleaner and more performant.

local -a displaysArr=("${(f@)displays}")
local -a valuesArr=("${(f@)values}")
local -a suffixesArr=("${(f@)suffixes}")

2. Grouping Completions by Suffix

Completions received from the Go binary are first grouped in an associative array based on their suffix type. This allows us to apply different compadd options to each group. A default value is used for completions with no suffix.

typeset -A grouped_values=()
typeset -A grouped_displays=()

for i in {1..${#valuesArr[@]}}; do
  local suffix_key="${suffixesArr[$i]:-__NOSUFFIX__}"
  grouped_values[$suffix_key]+="${valuesArr[$i]}"$'\n'
  grouped_displays[$suffix_key]+="${displaysArr[$i]}"$'\n'
done

3. Unified Group Display

To prevent _describe from creating a new visual group for each suffix type, a "first call" flag ensures the group title is only printed once.

local first_call=1
for suffix_key in "${(@k)grouped_values}"; do
  # ...
  local -a describe_args
  if (( first_call )); then
    describe_args=(-t "${tag}" "${tag}")
    first_call=0
  else
    describe_args=(-t "${tag}" "")
  fi
  # ...
done

4. Nuanced Suffix Removal

The script uses a robust if/elif/else structure to apply specific removal rules based on the suffix type, providing a highly intuitive user experience.

  • Separator Suffixes: For common separators like a space, /, ,, or :, the suffix is removed only if the user types another space.
  • Other Suffixes: For other characters that are part of a value (like =), the suffix is kept if the user continues typing letters or numbers.
  • No Suffix: Standard completion behavior is applied.
local separators=" /,.':@"
if [[ "$suffix_key" == "__NOSUFFIX__" ]]; then
  _describe "${describe_args[@]}" s_displays s_values -Q
elif [[ "$separators" == *"$suffix_key"* ]]; then
  _describe "${describe_args[@]}" s_displays s_values -Q -S "$suffix_key" -r ' '
else
  _describe "${describe_args[@]}" s_displays s_values -Q -S "$suffix_key" -r "0-9a-zA-Z"
fi

Performance Refactor

The initial block of the Zsh script, which handles complex shell quoting, has been refactored for performance and clarity.

  • Eliminated External Commands: The previous implementation relied on multiple calls to external commands like sed and xargs within a series of if/elif statements. These have been removed.
  • Native Zsh Features: The logic now uses Zsh's native array slicing (${words[1,-2]}) to manipulate the command-line words directly in memory, which is significantly faster than forking external processes.
  • Reduced Redundancy: The main call to the carapace binary is no longer repeated. The correct input string is determined first and stored in a variable, leading to a single, clean invocation.
local completion_input
if echo ${words}"''" | xargs echo 2>/dev/null > /dev/null; then
  completion_input="${words}''"
elif echo "${words[1,-2]} ${words[-1]}'" | xargs echo 2>/dev/null > /dev/null; then
  completion_input="${words[1,-2]} ${words[-1]}'"
else
  completion_input="${words[1,-2]} ${words[-1]}\""
fi

local lines
lines="$(echo "${completion_input}" | CARAPACE_COMPLINE="${words}" CARAPACE_ZSH_HASH_DIRS="$(hash -d)" xargs %v _carapace zsh)"

@coveralls
Copy link

Pull Request Test Coverage Report for Build 16667954420

Details

  • 51 of 72 (70.83%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 58.194%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/shell/zsh/action.go 0 21 0.0%
Totals Coverage Status
Change from base Build 16456110803: 0.1%
Covered Lines: 3228
Relevant Lines: 5547

💛 - Coveralls

@rsteube
Copy link
Member

rsteube commented Aug 1, 2025

I'll have a look at this, but handling the suffix internally has been rather intentional.

@maxlandon
Copy link
Contributor Author

maxlandon commented Aug 2, 2025

Hello,

I just pushed another change because I realized the ZSH snippet would call _describe too many times and that would slow down performance when many completions must be displayed. I optimized the code so that it does not happen anymore.

About prefix being handled internally: this PR does not really put this principle into question: only the application itself knows when suffices should trigger some sort of no-space behavior. The thing is, there are in fact many different types of no-space behavior, and the application code alone cannot devise nor tell ZSH how to remove the suffix.
Therefore, the goal of this PR was to enhance how the application code tells ZSH to handle a given completion suffix: the app knows, for instance, when we are completing a comma-separated list of files and directories, and so it can tell ZSH 1) which completions should have a removable suffix and 2) given which next character typed should this suffix be removed.
In turn, ZSH can act on the completion suffix being removed or not.

I've performed a bit of println-based benchmarking, and the added loop (Go code) for reclassifying values according to their suffix character class is negligible. As well, my last commit ensure ZSH does not have to perform many resource intensive operations.

The result is very good: between others, slash management for files is handled just like ZSH handles them natively.
Many other completion types work awesomely with this: user@machine hosts, list of flag arguments, short flags and no-opt-def-val flags, etc. Even default space suffix works amazingly well: it is automatically removed if another space is entered, or if a comma is typed while the completion is a list, but not removed if the completion is not, etc.

Failing tests: the 5 failing tests might be because of the changes in the first 5-10 lines of the ZSH snippet (removal of using sed/xargs), so changing these lines like I did might not be much needed...

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.

3 participants