Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teach dynamic fish completions to expand variables #5884

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krobelus
Copy link

In fish, the command line

cargo b --manifest-path $PWD/clap_complete/Cargo.toml --example

wrongly gets example-name completions from $PWD/Cargo.toml instead of
$PWD/clap_complete/Cargo.toml.

This is because fish's "commandline --tokenize" option produces tokens
like "$PWD/clap_complete/Cargo.toml", which clap cannot see through.

Starting in the upcoming fish version 4, there is a new option,
"--tokens-expanded"1 to output the expanded arguments, matching
what the program would see during execution.

If an expansion of a token like {1,2,3}{1,2,3} would produce too
many results, the unexpanded token is forwarded, thus falling back to
existing behavior. Similarly, command substitutions ("$()") present
on the command line are forwarded as-is, because they are not deemed
safe to expand given that the user hasn't agreed to running them.

Use this option if available, to fix the above example.

I have not checked if any existing clap user tries to expand variables
on their own. If this is a real possibility, we could let users
opt into the new behavior.

While at it, break up this overlong line. Add redundant semicolons,
in case a preexisting completion accidentally joins these lines with
spaces2, like

if set -l completions (COMPLETE=fish myapp 2>/dev/null)
	echo "$completions" | source
end

I have not updated clap_complete/tests/snapshots/register_dynamic.fish
(not sure how?) or tested this but I'm happy to do so.

Footnotes

  1. https://github.com/fish-shell/fish-shell/pull/10212

  2. https://github.com/fish-shell/fish-shell/pull/11046#discussion_r1917875912

In fish, the command line

	cargo b --manifest-path $PWD/clap_complete/Cargo.toml --example

wrongly gets example-name completions from $PWD/Cargo.toml instead of
$PWD/clap_complete/Cargo.toml.

This is because fish's "commandline --tokenize" option produces tokens
like "$PWD/clap_complete/Cargo.toml", which clap cannot see through.

Starting in the upcoming fish version 4, there is a new option,
"--tokens-expanded"[^1] to output the expanded arguments, matching
what the program would see during execution.

If an expansion of a token like {1,2,3}{1,2,3} would produce too
many results, the unexpanded token is forwarded, thus falling back to
existing behavior. Similarly, command substitutions ("$()") present
on the command line are forwarded as-is, because they are not deemed
safe to expand given that the user hasn't agreed to running them.

Use this option if available, to fix the above example.

I have not checked if any existing clap user tries to expand variables
on their own. If this is a real possibility, we could let users
opt into the new behavior.

While at it, break up this overlong line.  Add redundant semicolons,
in case a preexisting completion accidentally joins these lines with
spaces[^2], like

	if set -l completions (COMPLETE=fish myapp 2>/dev/null)
		echo "$completions" | source
	end

I have not updated clap_complete/tests/snapshots/register_dynamic.fish
(not sure how?) or tested this but I'm happy to do so.

[^1]: fish-shell/fish-shell#10212
[^2]: fish-shell/fish-shell#11046 (comment)
Comment on lines +222 to +233
r#"
complete --keep-order --exclusive --command {bin} --arguments \
"({var}=fish "'{completer}'" -- (
set --local tokenize;
if commandline --tokens-expanded >/dev/null 2>&1;
set tokenize --tokens-expanded;
else;
set tokenize --tokenize;
end;
commandline --current-process $tokenize --cut-at-cursor
) (commandline --current-token))"
"#
Copy link
Member

Choose a reason for hiding this comment

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

Could you split this into two commits

  1. Reformats the output
  2. Adds the new flag

Comment on lines +222 to +233
r#"
complete --keep-order --exclusive --command {bin} --arguments \
"({var}=fish "'{completer}'" -- (
set --local tokenize;
if commandline --tokens-expanded >/dev/null 2>&1;
set tokenize --tokens-expanded;
else;
set tokenize --tokenize;
end;
commandline --current-process $tokenize --cut-at-cursor
) (commandline --current-token))"
"#
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test for this?

@epage
Copy link
Member

epage commented Jan 20, 2025

In fish, the command line

cargo b --manifest-path $PWD/clap_complete/Cargo.toml --example

wrongly gets example-name completions from $PWD/Cargo.toml instead of $PWD/clap_complete/Cargo.toml.

A couple of cautions

If an expansion of a token like {1,2,3}{1,2,3} would produce too
many results, the unexpanded token is forwarded, thus falling back to
existing behavior.

I've been wondering about allowing completions for cases like file-name-{one,two}.[TAB] Would this change usually get in the way? The main way I see it getting in the way is dependent on what it does if no matches are present for it to expand the tokens.

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