Move most of the peculiar argument handling for external calls into the parser

[devyn]

Description

We've had a lot of different issues and PRs related to arg handling with externals since the rewrite of run-external in #12921:

• Fixes . expanded incorrectly as external argument #12950
• Do not propagate glob creation error for external args #12955
• 0.94 regression: Cannot invoke a binary with a path that starts with ~ #13000
• Restore tilde expansion on external command names #13001
• Tilde expansion for external command arguments #13021
• Fix external command name parsing with backslashes, and add tests #13027
• Fix run_external::expand_glob() to return paths that are PWD-relative but reflect the original intent #13028
• run_external: remove inner quotes once nushell gets = sign #13073

Many of these are caused by the argument handling of external calls and run-external being very special and involving the parser handing quoted strings over to run-external so that it knows whether to expand tildes and globs and so on. This is really unusual and also makes it harder to use run-external, and also harder to understand it (and probably is part of the reason why it was rewritten in the first place).

This PR moves a lot more of that work over to the parser, so that by the time run-external gets it, it's dealing with much more normal Nushell values. In particular:

• Unquoted strings are handled as globs with no expand
• The unescaped-but-quoted handling of strings was removed, and the parser constructs normal looking strings instead, removing internal quotes so that run-external doesn't have to do it
• Bare word interpolation is now supported and expansion is done in this case
  • Expressions typed as Glob containing Expr::StringInterpolation now produce Value::Glob instead, with the quoted status from the expr passed through so we know if it was a bare word
  • Bare word interpolation for values typed as glob now possible, but not implemented
• Because expansion is now triggered by Value::Glob(_, false) instead of looking at the expr, externals now support glob types

User-Facing Changes

• Bare word interpolation works for external command options, and otherwise embedded in other strings:

  ^echo --foo=(2 + 2) # prints --foo=4
  ^echo -foo=$"(2 + 2)" # prints -foo=4
  ^echo foo="(2 + 2)" # prints (no interpolation!) foo=(2 + 2)
  ^echo foo,(2 + 2),bar # prints foo,4,bar

• Bare word interpolation expands for external command head/args:

  let name = "exa"
  ~/.cargo/bin/($name) # this works, and expands the tilde
  ^$"~/.cargo/bin/($name)" # this doesn't expand the tilde
  ^echo ~/($name)/* # this glob is expanded
  ^echo $"~/($name)/*" # this isn't expanded

• Ndots are now supported for the head of an external command (^.../foo works)

• Glob values are now supported for head/args of an external command, and expanded appropriately:

  ^("~/.cargo/bin/exa" | into glob) # the tilde is expanded
  ^echo ("*.txt" | into glob) # this glob is expanded

• run-external now works more like any other command, without expecting a special call convention
  for its args:

  run-external echo "'foo'"
  # before PR: 'foo'
  # after PR:  foo
  run-external echo "*.txt"
  # before PR: (glob is expanded)
  # after PR:  *.txt

Tests + Formatting

Lots of tests added and cleaned up. Some tests that weren't active on Windows changed to use nu --testbin cococo so that they can work. Added a test for Linux only to make sure tilde expansion of commands works, because changing HOME there causes ~ to reliably change.

• 🟢 toolkit fmt
• 🟢 toolkit clippy
• 🟢 toolkit test
• 🟢 toolkit test stdlib

After Submitting

• release notes: make sure to mention the new syntaxes that are supported

[kubouch]

Awesome! I was going to suggest that we probably shouldn't be doing the external arg parsing in run-external, but it should be done in the parser.

[devyn]

Great! There are a few things I'd appreciate a review on. One in particular is how I handled string interpolations that should turn into globs - I just made them Expr::StringInterpolation with Type::Glob. I didn't really see any other expressions out there that turn into different values depending on their Expression type, so I'm wondering if this would ever cause a problem or if it's too weird and I should just add an Expr::GlobInterpolation or something like that instead.

This is my first time really digging into the nushell parser code and... well, it's not bad, it's easy enough to understand for me, but I can understand why we're trying to push forward the new parser. There are a lot of special cases and it's not as much based on building up smaller parsers as I would have hoped, and there aren't really a lot of parser combinator type helper functions either.

But I'm also probably not as familiar with general patterns there as I could be, so some more review of those changes would be appreciated too.

[fdncred]

This bare word string interpolation seems pretty epic to me 🚀. Reduces some typing.

What about single quotes and backtick quotes? I haven't tried this PR yet.

^$'~/.cargo/bin/($name)' # does this expand the tilde? assume no
^$`~/.cargo/bin/($name)` # does this expand the tiled? or maybe this isn't even possible. Not sure we have backtick interpolation yet. assume no or not even supported

[kubouch]

@fdncred IMO,

^$`~/.cargo/bin/($name)`

should expand tilde because it's a bareword. The first one shouldn't, I agree.

[devyn]

^$'~/.cargo/bin/($name)' # does this expand the tilde? assume no

Nope

^$`~/.cargo/bin/($name)` # does this expand the tiled? or maybe this isn't even possible. Not sure we have backtick interpolation yet. assume no or not even supported

We don't have that syntax yet I think but if we did it probably should.

[kubouch]

We don't have that syntax yet I think but if we did it probably should.

Then I don't understand when is the Expr::StringInterpolation(..., false) or the proposed Expr::GlobInterpolation used, I thought these are for

$`~/.cargo/bin/($name)`

[fdncred]

@devyn It would be good to change the default_config.nu since you added shape_glob_interpolation. The themes should have the default color in them for light and dark.

I realize this is mostly a run-external PR but I'm wondering if some internal commands need to be updated to support bare word string interpolation? Or maybe echo is the only one that is supposed to work (but without expansion) - not sure exactly.
For instance, this doesn't work, but I'm not sure if it's supposed to.

❯ let bin_name = "bin"
❯ ls ~/.cargo/($bin_name)
Error: nu::shell::directory_not_found

  × Directory not found
   ╭─[entry #2:1:4]
 1 │ ls ~/.cargo/($bin_name)
   ·    ──────────┬─────────
   ·              ╰── directory not found
   ╰────
  help: /Users/fdncred/.cargo/($bin_name) does not exist

but these work

❯ echo ~/.cargo/($bin_name)
~/.cargo/bin
❯ ^echo ~/.cargo/($bin_name)
/Users/fdncred/.cargo/bin
❯ ^ls ~/.cargo/($bin_name)
... all my bins ...

[devyn]

It would be good to change the default_config.nu since you added shape_glob_interpolation. The themes should have the default color in them for light and dark.

Sure, I'll do that.

I realize this is mostly a run-external PR but I'm wondering if some internal commands need to be updated to support bare word string interpolation? Or maybe echo is the only one that is supposed to work (but without expansion) - not sure exactly.

I didn't really do anything to make this work with internal commands yet. The complicated bit is that there are a bunch of different syntax shapes that are used. I think I can add it for SyntaxShape::Glob quite easily now that I've added GlobInterpolation, but it would probably take interpolation types for Filepath and Directory too.

Overall I'm not very satisfied with how this is currently designed and it feels like it should be more compositional, so we don't need many variants of the same expressions just to support interpolation