extern definitions that map to aliases can't be executed

[lukexor]

Describe the bug

I am loving the new way to provide custom completion sources - but I'm having trouble getting it to work when aliases overlap with the external command.

I have a alias cre = cargo run --example - the completion works fine, but it will only execute if you type out the full command (e.g. cargo run --example 2d_raycasting)

[13:07] v0.59.0 ❯ cre 2d_raycasting
Error: nu::shell::external_command (https://docs.rs/nu-protocol/0.59.0/nu-protocol/enum.ShellError.html#variant.ExternalCommand)

  × External command
   ╭─[entry #7:1:1]
 1 │ cre 2d_raycasting
   · ─┬─
   ·  ╰── can't run executable

How to reproduce

1. Define the following config:

alias cre = cargo run --example

def "nu-complete cargo bins" [] {
  let $bins = (ls src | where name =~ bin | each { |f| ls -s $f.name } | flatten | where name =~ .rs || type == dir)
  if ($bins | length) > 0 {
    echo $bins | update name { |file| $file.name | str find-replace ".rs" "" } | get name
  }
}

def "nu-complete cargo examples" [] {
  let $examples = (ls | where name =~ examples | each { |f| ls -s $f.name } | flatten | where name =~ .rs || type == dir)
  if ($examples | length) > 0 {
    echo $examples | update name { |file| $file.name | str find-replace ".rs" "" } | get name
  }
}

def "nu-complete cargo profiles" [] {
  open Cargo.toml | get profile | transpose | get column0
}

def "nu-complete cargo features" [] {
  open Cargo.toml | get features | transpose | get column0
}

extern "cargo run" [
  ...args: any                                   # arguments
  --bin: string@"nu-complete cargo bins"         # Name of the bin target to run
  --example: string@"nu-complete cargo examples" # Name of the example target to run
  --quiet(-q)                                    # Do not print cargo log messages
  --package(-p): string                          # Package with the target to run
  --jobs(-j): number                             # Number of parallel jobs, defaults to # of CPUs
  --release                                      # Build artifacts in release mode, with optimizations
  --profile: string@"nu-complete cargo profiles" # Build artifacts with the specified profile
  --features: string@"nu-complete cargo features"# Space or comma separated list of features to activate
  --all-features                                 # Activate all available features
  --no-default-features                          # Do not activate the `default` feature
  --target: string                               # Build for the target triple
  --target-dir: path                             # Directory for all generated artifacts
  --manifest-path: path                          # Path to Cargo.toml
  --message-format: string                       # Error format
  --unit-graph                                   # Output build graph in JSON (unstable)
  --ignore-rust-version                          # Ignore `rust-version` specification in packages
  --verbose(-v)                                  # Use verbose output (-vv very verbose/build.rs output)
  --color: string                                # Coloring: auto, always, never
  --frozen                                       # Require Cargo.lock and cache are up to date
  --locked                                       # Require Cargo.lock is up to date
  --offline                                      # Run without accessing the network
  --config: string                               # Override a configuration value (unstable)
  -Z: string                                     # Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details
  --help(-h)                                     # Prints help information
]

2. Type cre <Tab> in a cargo project with an examples/ folder
3. Select one of the examples with <Enter>
4. Press <Enter> again to execute the command

Expected behavior

Expected that cargo run --example <auto-completed-example> is executed

Instead:

Error: nu::shell::external_command (https://docs.rs/nu-protocol/0.59.0/nu-protocol/enum.ShellError.html#variant.ExternalCommand)

  × External command
   ╭─[entry #9:1:1]
 1 │ cre 2d_raycasting
   · ─┬─
   ·  ╰── can't run executable
   ╰────
  help: No such file or directory (os error 2)

Note that running cargo run --example 2d_raycasting works as expected and reproduce the above steps above by starting out typing cargo run --example <Tab> works as expected.

Screenshots

No response

Configuration

| key                | value                                    |
| ------------------ | ---------------------------------------- |
| version            | 0.59.0                                   |
| branch             | main                                     |
| short_commit       | f507613b                                 |
| commit_hash        | f507613b3866087d85034080bb2d7ec07fa928bc |
| commit_date        | 2022-02-22 16:32:29 +00:00               |
| build_os           | macos-x86_64                             |
| rust_version       | rustc 1.58.1 (db9d1b20b 2022-01-20)      |
| rust_channel       | stable-x86_64-apple-darwin               |
| cargo_version      | cargo 1.58.0 (f01b232bc 2022-01-19)      |
| pkg_version        | 0.59.0                                   |
| build_time         | 2022-02-22 19:42:25 -08:00               |
| build_rust_channel | release                                  |
| features           | default, which                           |
| installed_plugins  |                                          |

Additional context

This also doesn't work for normal commands either (not involving flags). Using the default config get checkout completion with an alias of alias gco = git checkout results in a similar failure:

[13:35] v0.59.0 ❯ gco main
Error: nu::shell::external_command (https://docs.rs/nu-protocol/0.59.0/nu-protocol/enum.ShellError.html#variant.ExternalCommand)

  × External command
   ╭─[entry #4:1:1]
 1 │ gco main
   · ─┬─
   ·  ╰── can't run executable
   ╰────
  help: No such file or directory (os error 2)

[sophiajt]

I think we need to add aliases to the complete_commands function in completions.rs. With that, I think that should be enough to start to show up.

Another piece is that both alias and use seem to have some issues with completions generally. I'm not sure what exactly is going on, but hopefully once that's fixed it'll help.

For the cases you show where it's failing to run the executable, I'd like to see if we can dig into why. Aliases shouldn't have any particular restrictions here, but perhaps they aren't playing well with subcommands?

[lukexor]

So in my testing - The only thing that makes the alias break is the definition of extern for the aliased command.

For example:

[14:17] v0.59.0 ❯ alias e = echo
[14:17] v0.59.0 ❯ e "hi"
hi
[14:17] v0.59.0 ❯ extern "echo" [str: string]
[14:17] v0.59.0 ❯ e "hi"
Error: nu::shell::external_command (https://docs.rs/nu-protocol/0.59.0/nu-protocol/enum.ShellError.html#variant.ExternalCommand)

  × External command
   ╭─[entry #9:1:1]
 1 │ e "hi"
   · ┬
   · ╰── can't run executable
   ╰────
  help: No such file or directory (os error 2)
[14:17] v0.59.0 ❯ echo "hi"
hi

[sophiajt]

For folks who want to look into this one -

I think this is coming from a hack that we're doing for KnownExternal. In Nushell, we currently don't have a way to retain the original order of named/positional parameters that the user gave us. For internal commands, this is fine. For external commands, they'll have differing ways of handling flags and positional parameters. We need to retain the exact order the user gave us originally.

Right now, in order to retain this original order, we re-parse the expression as an external command. For simple forms, this is fine. For forms like the above, where we need to expand an alias to get to the original, it's not going to work.

There are probably a few ways to fix this. My hunch is that we may want to parse it both as an internal command, so that we can do typechecking and completions, and as an external command (both using alias expansion).

In the future, perhaps we'll have a way to denote in the signature that the command needs to retain its original order.

[fdncred]

@lukexor unrelated to this issue, these are really cool custom completions. we should put them in the nu_scripts repo somewhere like under engine-q/completions or something.

[chtenb]

I just also ran into this bug, but it seems you can work around it by prepending the external command with ^.

[lukexor]

Any update on design ideas here? I really want to make use of the completion support, but there's no way I can type the full commands in my day to day for it.

Though I just had a thought that I could work around it by just defining the completions on my aliases instead, albeit there may be some duplication.

Edit: Tried the above, but I don't think it's possible.

[lukexor]

~/dev/nushell on  main is 📦 v0.61.0 via 🦀 v1.59.0
[20:54] v0.61.0 ❯ which gco
╭───┬─────┬─────────────────────────────┬──────────╮
│ # │ arg │            path             │ built-in │
├───┼─────┼─────────────────────────────┼──────────┤
│ 0 │ gco │ Nushell alias: git checkout │ false    │
╰───┴─────┴─────────────────────────────┴──────────╯

~/dev/nushell on  main is 📦 v0.61.0 via 🦀 v1.59.0
[20:54] v0.61.0 ≡ gco<TAB>
main                                                       remotes/origin/HEAD -> origin/main                         remotes/origin/engine-q                                    remotes/origin/jntrnr-patch-1
remotes/origin/kubouch-patch-1                             remotes/origin/main                                        remotes/origin/revert-4835-alias_sources

The completion works, but actual execution fails so tab-completion seems to know the alias is associated to the extern command, but the execution parser isn't.

Error: nu::shell::external_command (https://docs.rs/nu-protocol/0.61.0/nu_protocol/enum.ShellError.html#variant.ExternalCommand)

  × External command
   ╭─[entry #19:1:1]
 1 │ gco remotes/origin/engine-q
   · ─┬─
   ·  ╰── can't run executable
   ╰────
  help: No such file or directory (os error 2)

[lukexor]

I'm trying to look into this myself to give more insight - so far the most I've been able to gather is that potentially there is a span mismatch going on.

The Call, Pipeline, and Spanned being evaluated for git checkout with a defined extern "git checkout" {} block is:

Call { decl_id: 305, head: Span { start: 37283, end: 37295 }, arguments: [], redirect_stdout: true, redirect_stderr: false }

Pipeline { expressions: [Expression { expr: Call(Call { decl_id: 305, head: Span { start: 37283, end: 37295 }, arguments: [], redirect_stdout: true, redirect_stderr: false }), span: Span { start: 37283, end: 37295 }, ty: Any, custom_completion: None }] }

Spanned { item: "git", span: Span { start: 37283, end: 37295 } }

Here the decl_id of 305 corresponds to the extern "git checkout" {} declaration and the span is the correct 12 characters for git checkout.

The Call, Pipeline and Spanned for the alias of gco is:

Call { decl_id: 305, head: Span { start: 14053, end: 14065 }, arguments: [], redirect_stdout: true, redirect_stderr: false }

Pipeline { expressions: [Expression { expr: Call(Call { decl_id: 305, head: Span { start: 37295, end: 37298 }, arguments: [], redirect_stdout: true, redirect_stderr: false }), span: Span { start: 37295, end: 37298 }, ty: Any, custom_completion: None }] }

Spanned { item: "gco", span: Span { start: 37295, end: 37298 }

The same decl_id is properly expanded from the alias evaluation, and the initial span is correct - but when it's converted to a Pipeline, the span is altered to be 3 corresponding to the three letter alias. I'm not really familiar enough with the parser to figure a solution that doesn't potentially break other features - but I'm going to keep looking

[lukexor]

So I found that if I comment out this line in parser.rs, it works as expected:

979                  result.replace_span(working_set, expansion_span, orig_span);

Though I'm not clear on what impact this has or why you'd want to replace an alias expansion span with the original pre-expanded span. If someone can advise - I'll be happy to put up a PR

Related: #4474

Note I went through and tried the steps in JT's screenshots and they seem to work fine without replacing the spans.

All tests also pass, but #4473 is still an issue

[lukexor]

I might be missing something, but it seems like spans are doing double duty - as both an execution reference for declaration blocks as well as for error messaging - which would definitely get tricky with alias expansion being involved. I'd expect having decl_id defined would mean you could look up the accurate span info for it, separate from the span used to expand/resolve that specific decl_id

I tried looking into a different way to solve #4474 - The output I keep getting is that additional expressions are being added to the pipeline that are outside of the range of the initial expansion.

> alias open = ^start
> open <TAB> 

The expression expansion parsing shows "start" followed by an "a" string expression - The span for which is outside of the range of the expansion span. By replacing the expansion span with the original, this "a" never shows up in the arguments for expression parsing.

e.g.

Expression { expr: ExternalCall(Expression { expr: String("start"), span: Span { start: 37301, end: 37306 }, ty: String, custom_completion: None }, [Expression { expr: String("a"), span: Span { start: 37311, end: 37312 }, ty: String, custom_completion: None }]), span: Span { start: 37300, end: 37312 }, ty: Any, custom_completion: None }

vs.

Expression { expr: ExternalCall(Expression { expr: String("start"), span: Span { start: 37302, end: 37307 }, ty: String, custom_completion: None }, []), span: Span { start: 37301, end: 37307 }, ty: Any, custom_completion: None }

[filaretov]

This and #5179 seem to be related.

@lukexor, as you rightly point out, line 979, where we replace the span is problematic. For external commands, we rely on getting the span contents to know which command to run. By replacing the new span with the original, we end up passing the alias to run-external.

It gets a bit trickier with subcommands. In the issue I linked, the user is trying to alias g = git and then use g push. This fails because during alias expansion we create a Call that has a head span that starts where alias g = git was defined (the aliased part of it) and ends on the last line the user entered (at the end of push). We can not get the content for this span, so in these situations we get a panic. That's what I think, at least.

It seems to me that a hacky solution would be to try to expand aliases again in known_external.rs. This wouldn't work with subcommands because of the weird span described above. If we could somehow circumvent that, maybe it'd work. A straightforward solution might be to have Call.head be a sequence of spans, but I don't know if this has other implications. Perhaps we should reconsider how aliasing works to solve it more cleanly.

I suspect the way we expand aliases is also related to the unhelpful error messages (#4965).

[filaretov]

@lukexor Does this PR fix the issues you've been experiencing?

#5213

[lukexor]

@filaretov it does! Thanks so much - I can't believe how simple the fix was!

[filaretov]

@lukexor Yeah, I was pleasantly surprised as well 😁