I'm in favor of having a way to get the expanded tokens, but I'm not sure

1. this is correct in general for all uses of commandline --tokenize - consider bindings
2. this is okay for completions that currently use eval - like our git completions (tho they do try to avoid some obvious issues)

The latter one is what you're aluding to with if a variable expansion contains spaces or other shell metacharacters, "eval" will do the wrong thing.

The change here is that now you get double-expansion - $bar"foo" with $bar set to (echo hahaha) will turn into (echo hahaha)foo and then pass to eval and actually run the echo hahaha that wasn't run before.

The obvious alternative is to add a second option, commandline --expanded-tokens, or commandline --expand --tokenize.

I'm in favor of having a way to get the expanded tokens, but I'm not sure

1. this is correct in general for all uses of commandline --tokenize - consider bindings

I don't think we can find a case where the current behavior is more useful.

2. this is okay for completions that currently use eval - like our git completions (tho they do try to avoid some obvious issues)

The latter one is what you're aluding to with if a variable expansion contains spaces or other shell metacharacters, "eval" will do the wrong thing.

The change here is that now you get double-expansion - $bar"foo" with $bar set to (echo hahaha) will turn into (echo hahaha)foo and then pass to eval and actually run the echo hahaha that wasn't run before.

I think eval (commandline -opc) is not a very common idiom because eval is bad practice here. It was always a workaround.
Completions that use eval were already vulnerable but yeah, this change amplifies it.
But pushing people to get off eval for completions is a good thing.

The obvious alternative is to add a second option, commandline --expanded-tokens, or commandline --expand --tokenize.

FWIW, if we want two modes, then, besides an expansion mode we could have one that gives raw tokens (without today's bogus unescaping). But I wouldn't add that without a use case and I don't think we can find one.

My hope is that changing existing behavior will be a net positive because it gives the improved behavior to third party completions for free.
Most "forwarding" uses of commandline -opc do not call eval but forward them as unexpanded argv, like myprogram complete (commandline -opc).
The expansion behavior is a strict improvement here.
For example Rust's clap library provides dynamic completions with

r#"complete -x -c {bin} -a "("'{COMPLETER}'" complete --shell fish -- (commandline --current-process --tokenize --cut-at-cursor) (commandline --current-token))""#

Essentially everything that is broken by this change was already broken in similar ways; we can fix those first.
So far we only know of Cobra. Once they merge https://github.com/spf13/cobra/pull/2095/files, their logic will be safe both before and after this change.

I'm not really sure how to search for third party code. What command line completion frameworks other than clap/Cobra, or other notable third party scripts can we find?

What command line completion frameworks other than clap/Cobra, or other notable third party scripts can we find?

The pip completions also eval the output of commandline -o.

(for the record, pipenv does not, at least the version I have)

rclone currently does, but it uses cobra. I'm not sure what updating them would entail.

stack doesn't, starship doesn't.

Those are the first five of our | source completions I could get my hands on.

I prefer to break users here (short sharp shock) but it sounds like there's an objection so I'll add a separate flag?

I prefer to break users here (short sharp shock) but it sounds like there's an objection so I'll add a separate flag?

I would prefer that, yes. The issue is that it wouldn't be a "short sharp shock", it would appear to work in most cases, and then in edge cases you get a face full of eval.

And that's not just for completion generation projects, but also e.g. users who keep a modified version of our own git completions.

Note: For bindings we probably need a "raw" mode, where you get the full token with quotes and such intact.

Otherwise if you try to make a binding act on tokens it will be wrong as soon as any quotes enter the picture.

Now we have three possible modes, which makes me think we might want it to be one option with arguments.

Technically it's three but the current behavior should never be needed, I'd hide it from documentation, only mention it in the changelog.

I don't know of a use case for getting the list of raw tokens but it does make much more sense than unescaped ones.
I wonder how bindings would want to use them.
commandline -ct is raw but that's kind of unavoidable. Orthodox completion scripts don't look at that token.. but sometimes it's a necessity.

I realized read --tokenize is weird in the same way as commandline --tokenize; it does unescaping but leave variables.
We should probably update it the same way (add a new flag).

I don't know of a use case for getting the list of raw tokens but it does make much more sense than unescaped ones.

"Insert the last token again".

Currently my best attempt is

function last_arg
    set -l arg (commandline -opc)[-1]
    commandline -i -- $arg
end

bind \cg last_arg

but that fails if the last token is quoted, like "foo bar". Add string escape and then it'll turn it into 'foo bar' with single-quotes.

Generally any time you want to iterate over the commandline to transform it you need it not unescaped.

I realized read --tokenize is weird in the same way as commandline --tokenize; it does unescaping but leave variables.

read --tokenize is mainly useful for reading quotes - turn "foo bar" baz into foo bar and baz. Picture reading from an ini-style file, or we use it for figuring out $EDITOR
or $PAGER, where you might have escaped spaces (non-standard but they do work in some tools, I think systemd had some pretty complicated logic here).

In those cases you typically do not want variables and globs to be expanded.

both makes sense, thanks.
So commandline flags --expand-tokens and --raw-tokens or --tokens=expanded and --tokens=raw. But if we do the latter -x should still be an abbreviation for --tokens=expanded

So commandline flags --expand-tokens and --raw-tokens or --tokens=expanded and --tokens=raw. But if we do the latter -x should still be an abbreviation for --tokens=expanded

The latter sounds fine. Closest short opt we can do for "raw" is -R or -w.