str expand doesn't like backslashes for path separators

[fdncred]

Describe the bug

@atahabaki I think we just found a bug? in str expand. Would you mind taking a look at it?

On Windows, the path separator is \. If we do this:

"crates" | path join "nu-utils" "src" "sample_config" "default_{config,env}.nu"
crates\nu-utils\src\sample_config\default_{config,env}.nu

all looks good until we add str expand, which gives us this:

❯ 'crates' | path join 'nu-utils' 'src' 'sample_config' 'default_{config,env}.nu' | str expand
╭───┬─────────────────────────────────────────────────╮
│ 0 │ cratesnu-utilssrcsample_configdefault_config.nu │
│ 1 │ cratesnu-utilssrcsample_configdefault_env.nu    │
╰───┴─────────────────────────────────────────────────╯

What I expect is the same output as this.

❯ [crates nu-utils src sample_config 'default_{config,env}.nu'] | str join '\\' | str expand
╭───┬─────────────────────────────────────────────────────╮
│ 0 │ crates\nu-utils\src\sample_config\default_config.nu │
│ 1 │ crates\nu-utils\src\sample_config\default_env.nu    │
╰───┴─────────────────────────────────────────────────────╯

Thoughts? Maybe this is the correct behavior? I'm not sure. It makes dealing with Windows paths difficult.

How to reproduce

see above

Expected behavior

see above

Screenshots

No response

Configuration

key	value
version	0.83.1
branch	main
commit_hash	8403fff
build_os	windows-x86_64
build_target	x86_64-pc-windows-msvc
rust_version	rustc 1.69.0 (84c898d65 2023-04-16)
rust_channel	1.69.0-x86_64-pc-windows-msvc
cargo_version	cargo 1.69.0 (6e9a83356 2023-04-12)
build_time	2023-07-27 14:57:05 -05:00
build_rust_channel	release
allocator	mimalloc
features	dataframe, default, extra, sqlite, trash, which, zip
installed_plugins	from eml, from ics, from ini, from parquet, from plist, from vcf, gstat, highlight, hist, inc, json path, periodic-table, plot, pnet, query, query json, query web, query xml, regex, xyplot

Additional context

No response

[atahabaki]

I'm sorry, it's actually not a bug. bracoxide ignores what's came after the backslash. Sorry, I forgot to add that to the examples.

Take a look at this example, (I'm also updating the examples).

'A{B\,,C}' | str expand

bracoxide (I implemented this feature intentionally) walks through the chars, sees there is a \, ignores the next character as well if the next character is not a backslash again. So it produces:

╭───┬─────╮
│ 0 │ AB, │
│ 1 │ AC  │
╰───┴─────╯

another example:

'A{B\\,C}' | str expand

produces:

╭───┬─────╮
│ 0 │ AB\ │
│ 1 │ AC  │
╰───┴─────╯

So, to solve the problem, we left with (make the backslashes double algorithm):

'crates' | path join 'nu-utils' 'src' 'sample_config' 'default_{config,env}.nu'  | str replace '\\' '\\\\'| str expand

I don't have an idea, should bracoxide crate just handle escaping braces ({ or }), commas (,), and dots .? I don't have a solid idea? What do you think? I can easily change it. Just a couple of lines, and it will just escape the special characters. Thanks!

[fdncred]

hmmm, it seems that str expand and str bexpand both want to work similarly, by escaping backslashes. I wonder if the right thing to do is to add a parameter to path join like path join --escape?

[atahabaki]

I think I might change how bracoxide handles the escaping. Let's say the piped data is A\n{B,C} it skips the meaningful data new-line char. Is there a list of backslash characters list in nushell? Yeah, I think it's better to update bracoxide, I think I'm going to update the crate again. 😆

[atahabaki]

As for the, windows side part, should not be a problem unless it is:
C:\Users\atahabaki\Desktop\new folder?

See the \n? 😅 I think if I update it as I said in here (#9838 (comment)), will assume that it's newline. Should be a better approach. I think the double-backslash is the best.

[fdncred]

Windows is a mess with the stupid backslash. It always has been. I think this is where nushell handles this type of escaping. I almost added some more the other day.

[atahabaki]

Windows is a mess with the stupid backslash.

I agree, that's why I don't use Windows. 😅 It's still a pain in the back to set up a fully functional development environment on Windows. That's exactly why I switched to Linux 6+ years ago.

The stupid Windows. Makes everything difficult.

I think the best approach to this would be path escape a new subcommand if the user in Windows? Or a flag in realated path subcommands --escape. I don't know.

Cause if I change how the bracoxide handles the escaping. It's still a mess on Windows if the folder starts with 'a', 'b', 'e', 'f', 'n', 'r', 't', 'u'. Then the user should replace \n with \\n (or corresponding one), that's much greater mess. Current implementation is I assume and think it's the best.

I can create a PR for path join --escape.

[fdncred]

I can create a PR for path join --escape.

That would be great! Thanks for the help!

[atahabaki]

Hi, I made changes to path join, I actually need your feedback on #9848.

I may add the --escape flag to all path subcommands. For better solution, I need to modify PathBuf or I need to create a structure like PathBuf to pass a flag…

[sholderbach]

I am suspicious of the claim that the solution is to make changes to one path command, what happens if you proceed with a different combination of path commands or pass to external or internal tools expecting a path.
To me the question is first: is the grammar of str expand compatible with the complexity of nu's different string types with different quoting and escaping rules?

[atahabaki]

I am suspicious of the claim that the solution is to make changes to one path command, what happens if you proceed with a different combination of path commands or pass to external or internal tools expecting a path.

I can create a flag to str expand, such as --path?

So below command

['C:', '\', 'Users', 'viking', "{spam,welcome}.txt"] | path join --escape | str expand 

turns into

['C:', '\', 'Users', 'viking', "{spam,welcome}.txt"] | path join | str expand --path

Which is fine for me, I'll do exactly the same operation, replace all single backslashes with the double backslashes.

To me the question is first: is the grammar of str expand compatible with the complexity of nu's different string types with different quoting and escaping rules?

bracoxide crate is open for contributions. I had 2 ideas for skipping:

1. backslashes,
2. doubling the operator, i.e., to bypass comma use ,,, to bypass opening braces use {{ which sounds great but take a look at this: Jhonny says {{{hi,goodbye}}}. produces 1. Jhonny says {hi}, 2. Jhonny says {goodbye}. The more it gets complex, the readability gets much more worse.

I couldn't find any more ideas than these. My honest thought clarifies that the first option which is using backslashes by far the best option. Just in case, I'll pretty soon create a PR for --path flag too.

[fdncred]

From my perspective, I'm fine with either str expand --path or path join --escape as both fix the Windows problem. I do have a question about escaping spaces though. IIRC, nushell doesn't allow spaces to be escaped like \ , but I could be remembering wrong. Also, space escaping, if allowed, would probably need to work on Windows, MacOS, and Linux, right?

I'm also fine with deferring where to put the command to any core-team member, like @sholderbach, who fervently believes it needs to be one or the other. Either is fine as long as it fixes the problem without having to do str replace manually.