Skip to content

Escape .list completion candidates for fish - #966

Open
dylanpulver wants to merge 1 commit into
apple:mainfrom
dylanpulver:fish-list-completion-escaping
Open

Escape .list completion candidates for fish#966
dylanpulver wants to merge 1 commit into
apple:mainfrom
dylanpulver:fish-list-completion-escaping

Conversation

@dylanpulver

Copy link
Copy Markdown

complete -a re-parses its argument as fish source, so .list candidates went in raw. A candidate containing ' ends the surrounding quoted string and the whole generated script fails to parse — the tool then has no fish completions at all.

Fix: emit each candidate as a single-quoted fish token, escaped twice — the idiom the .file case in this same switch already uses. #811 did this for shellCommand; .list was left behind.

Measured on fish 4.8.1 with a 10-value .list including it's, two words, back\slash, $HOME, * and (echo PWNED); three clean-scratch builds:

parses offered
main no — quotes are not balanced 0/10
escaping once yes 0/10 — complete rejects it: contains a syntax error
this PR yes 10/10 exact; $HOME, *, (...) not expanded

Generated bash and zsh scripts are byte-identical to main in all three builds.

swift test: 562/562 with the fix; 5 failures on main and on the escape-once variant with the new fixture present.

Not addressed here: bash drops the ' from such a candidate (it'sits). Different code path, unchanged by this PR.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

Fish re-parses the argument to `complete -a` as fish source, so a
candidate containing a single quote ended the surrounding quoted string
and made the whole generated script unparseable. Escape each candidate
the way the `.file` case already does: emit it as a single-quoted fish
token, escaped twice.

This also stops candidates that contain spaces from being split into
several candidates, and stops `$var`, globs and `(...)` from being
expanded, matching the documented contract on `CompletionKind.list`
that candidates are interpreted as literals.
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.

1 participant