Skip to content

Fix zsh compadd shim dropping descriptions from _describe's clustered -ld - #15313

Draft
warp-agent-staging[bot] wants to merge 1 commit into
masterfrom
factory/zsh-compadd-describe-flag-fix
Draft

Fix zsh compadd shim dropping descriptions from _describe's clustered -ld#15313
warp-agent-staging[bot] wants to merge 1 commit into
masterfrom
factory/zsh-compadd-describe-flag-fix

Conversation

@warp-agent-staging

Copy link
Copy Markdown
Contributor

Summary

The zsh completions compadd shim silently drops every description that comes from _describe.

app/assets/bundled/bootstrap/zsh_body.sh located the description array with an exact-match index lookup, $@[(I)-d]. _describe never passes that flag on its own — it passes it clustered with other short flags, as -ld. The lookup therefore returned 0, no description array was resolved, and every match produced through _describe came back with an empty description.

_arguments-based option descriptions, which do pass -d unclustered, were unaffected. That is why the bug is easy to miss: some completions have descriptions and others silently do not.

Fix

Match any flag token consisting of a leading -, zero or more letters and a trailing d, instead of requiring an exact -d.

Two details worth calling out:

  • The presence test keeps (I), not (i). ${@[(i)pattern]} returns $# + 1 rather than 0 when nothing matches, which would make the test true on every compadd call and resolve an empty array name.
  • The search is restricted to the leading flags-only prefix — everything before the first bare -/-- — exactly as the neighboring -O/-A/-D check already does. Without that, the lookup scans the candidate words too, so a completion candidate that happens to look like a flag (a literal -d or -ld, as ls and find offer) would be taken as the description flag and the following candidate expanded as an array name.

Verification

Verified in a real zsh 5.9: git ch previously yielded matches with empty descriptions, and now yields checkout → "checkout branch or paths to working tree", cherry-pick → "apply changes introduced by some existing commits", and so on for all eight matches. Also checked against argument lists with no -d at all, with a plain -d, with a clustered -ld, and with candidates that themselves look like flags.

The affected code is reachable only behind FeatureFlag::NativeShellCompletions, which is off on every channel, so there is no user-visible impact today. It is worth fixing before that flag is promoted, and it is independent of any change to how completions are triggered.

Stack

This is the first PR of a two-PR stack and should merge first:

  1. this PR — the compadd shim description fix (CORE-3795)
  2. #15294 — generator-based native shell completions for all four shells (CORE-3794), based on this branch

#15294 depends on this fix, since its zsh path relies on descriptions being resolved correctly.

… -ld

The completions `compadd` shim located the description array with an exact
match on `-d`, but `_describe` never passes that flag on its own -- it passes
it clustered with other short flags, as `-ld`. The lookup therefore found
nothing, no description array was resolved, and every match produced via
`_describe` came back with an empty description. `_arguments`-based option
descriptions, which do pass `-d` unclustered, were unaffected, which is why
some completions had descriptions and others silently did not.

Match any flag token of a leading `-`, zero or more letters and a trailing
`d` instead of requiring an exact `-d`, and keep using `(I)` rather than
`(i)`: `(i)` returns one past the end instead of 0 when nothing matches,
which would make the presence test true on every call. The search is also
restricted to the leading flags-only prefix the neighboring `-O`/`-A`/`-D`
check already uses, so a completion candidate that happens to look like a
flag -- a literal `-d` or `-ld`, as `ls` and `find` offer -- is never
mistaken for the flag itself.

Fixes CORE-3795.
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

This PR was generated with Warp.

Comment @warp-factory on this PR to send it follow-up work.

View run View conversation View on Slack

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants