Try to make argument with quotes for external command better

[WindSoilder]

Description

Fixes: #6399

Detail

In general, if one external arg is quoted, nu won't remove inner extra quotes.

Else, if one external arg is not quoted, nu will try to remove inner extra quotes, which is implemented in #5846

Summarized:

1. ^echo --aa="bbbb" ==> --aa=bbbb
2. ^echo "--aa='bbbb'" ==> --aa='bbbb'
3. ^echo '--aa="bbb"' ==> --aa="bbb"

For complex argument mentioned in #6399 (comment):

let dump_command = "PGPASSWORD='db_secret' pg_dump -Fc -h 'db.host' -p '$db.port' -U postgres -d 'db_name' > '/tmp/dump_name'"

If we want to use $dump_command as an argument, we can't use bare $dump_command, because it's intepreted as:

PGPASSWORD='db_secret' pg_dump -Fc -h 'db.host' -p '$db.port' -U postgres -d 'db_name' > '/tmp/dump_name'

We need to surrand $dump_command with ", and using them with String interpolation.

^echo $""($dump_command)""

Ok...have to admit it's a bit annoying

or just

^echo "PGPASSWORD='db_secret' pg_dump -Fc -h 'db.host' -p '$db.port' -U postgres -d 'db_name' > '/tmp/dump_name'"

Tests

Make sure you've done the following:

• Add tests that cover your changes, either in the command examples, the crate/tests folder, or in the /tests folder.
• Try to think about corner cases and various ways how your changes could break. Cover them with tests.
• If adding tests is not possible, please document in the PR body a minimal example with steps on how to reproduce so one can verify your change works.

Make sure you've run and fixed any issues with these commands:

• cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
• cargo clippy --workspace --features=extra -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
• cargo test --workspace --features=extra to check that all the tests pass

[fdncred]

let's try this to see if it helps folks with quoting.

[fdncred]

@WindSoilder do you know if these changes were reverted somewhere? I can't get this stuff working right anymore.

https://gist.github.com/fdncred/c55a7a83f41bb6b3aa48381dd44ed4f8
like this is broken

and this is broken

[fdncred]

oof, this test doesn't work any longer, in Windows:

^echo '--aa="bbb"'
--aa=\bbb"

[WindSoilder]

oof, this test doesn't work any longer:

^echo '--aa="bbb"'
--aa=\bbb"

This works for me in the latest main.

let selected_log_lines = (
git log [--format=format:($log_fmt_str) --date=short --color=always] |
fzf [--ansi --multi --tiebreak=index --preview='git show --color=always --stat --patch {1}']
)

For something like this, because we pass list to fzf, I think we need to quote the whole --preview='gitxxx'], like this:

let selected_log_lines = (
git log [--format=format:($log_fmt_str) --date=short --color=always] |
fzf [--ansi --multi --tiebreak=index --preview='git show --color=always --stat --patch {1}']
)

[fdncred]

I was testing on windows. This works on Mac but not Windows.

^echo '--aa="bbb"'

neither of the selected_log_lines examples work on my Mac on the latest main.

[WindSoilder]

neither of the selected_log_lines examples work on my Mac on the latest main.

I'm not sure if it's introduced by this pr, I'll install fzf and try on mac today.

[WindSoilder]

Hi @fdncred , there are two ways to work around this:

1. pass list to fzf:

git log [--format=format:($log_fmt_str) --date=short --color=always] | 
fzf [--ansi --multi --tiebreak=index  "--preview=git show --color=always --stat --patch {1}"]

We need to "--preview=git show --color=always --stat --patch {1}", rather than --preview="git show --color=always --stat --patch {1}"

If we only quote from git show part, it will show a string git show --color=always --stat --patch {1}, because the quote " is also passed to fzf command.

The rule is commented here:

nushell/crates/nu-command/src/system/run_external.rs

Lines 77 to 86 in 8564c53

	Value::List { vals, .. } => {
	// turn all the strings in the array into params.
	// Example: one_arg may be something like ["ls" "-a"]
	// convert it to "ls" "-a"
	for v in vals {
	spanned_args.push(value_as_spanned(v)?);
	// for arguments in list, it's always treated as a whole arguments
	arg_keep_raw.push(true);
	}
	}

2. don't pass list to fzf:

git log [--format=format:($log_fmt_str) --date=short --color=always] | 
fzf --ansi --multi --tiebreak=index  --preview='git show --color=always --stat --patch {1}'

Could you please check if it works for you?

[WindSoilder]

And,

oof, this test doesn't work any longer:

^echo '--aa="bbb"'
--aa=\bbb"

Here is an issue mentioned about this #6465, I'm sorry my computer is broken, I can't verify if it was introduced by this pr.

[fdncred]

Could you please check if it works for you?

Both of your solutions work in Mac. I'll test in Windows in a little while.

[fdncred]

Both of those work on Windows too. I'd still like to fix this problem though.

This does not work on windows

^echo '--aa="bbb"'
--aa=\bbb"

[evelant]

As a new nu user I was bitten by this. Very unexpected behavior. I was just trying to use ripgrep and this happened:

rg -e "npm_package_[\"'`]" -. --no-ignore --no-messages -t js -t ts -t json -M 100 ~/
regex parse error:
    npm_package_[\"'`]

From my perspective as a newbie that looks like "escape sequences don't work". Maybe a better solution than trying to cleverly handle quotes could be to add a custom string delimiter option so users can pick a delimiter that won't conflict with whatever they need to put in a string. That or have add string delimiter option that's very unlikely to conflict with anything such as ,,,string with "all sorts" of 'quotes', and `no conflicts`.,,,