Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to pass variable length of arguments to other commands #7758

Closed
r12f opened this issue Jan 15, 2023 · 25 comments
Closed

How to pass variable length of arguments to other commands #7758

r12f opened this issue Jan 15, 2023 · 25 comments
Labels
question the issue author asks something

Comments

@r12f
Copy link

r12f commented Jan 15, 2023

Question

Hi, I really like nushell and would like to use it for my daily shell, but I met some problems when I am trying to create some aliases in nushell, specifically - how to create an alias that accepts variable parameters. Could anyone help me take a look?

It might be a dumb question, but I am kinda stuck. I did some searches and have tried several ways below to do it, but none of them works.

First, I tried to use alias keyword (https://www.nushell.sh/book/aliases.html#aliases):

C:\Users\r12f\Downloads〉alias ll = (ls -l)                                                                                                                                                01/14/2023 10:56:56 PM
C:\Users\r12f\Downloads〉ll *.zip                                                                                                                                                          01/14/2023 11:00:21 PMError: nu::parser::parse_mismatch (link)

  × Parse mismatch during operation.
   ╭─[entry #58:1:1]
 1 │ ll *.zip
   ·    ──┬──
   ·      ╰── expected operator
   ╰────

Then I have tried to use custom commands with rest parameters (https://www.nushell.sh/book/custom_commands.html#rest-parameters):

C:\Users\r12f\Downloads〉def lsl [...x] { ls -l $x }                                                                                                                                       01/14/2023 10:55:56 PMC:\Users\r12f\Downloads〉lsl *.zip                                                                                                                                                         01/14/2023 10:56:52 PM
Error: nu::shell::cant_convert (link)

  × Can't convert to string.
   ╭─[entry #55:1:1]
 1 │ def lsl [...x] { ls -l $x }
   ·                        ─┬
   ·                         ╰── can't convert list<string> to string
   ╰────

With some searches, I found a few threads discussing similar things (e.g. #1716), so I have also tried the syntax there, but it looks like the syntax is already changed:

C:\Users\r12f\Downloads〉alias ll [x...] { ls -l $x }                                                                                                                                      01/14/2023 11:00:23 PMError: nu::parser::assignment_mismatch (link)

  × alias missing sign
   ╭─[entry #59:1:1]
 1 │ alias ll [x...] { ls -l $x }
   ·          ───┬──
   ·             ╰── missing equal sign
   ╰────

So I am wondering what would be the right way to do it and is there any doc for this?

Really appreciate for the help in advance!

Additional context and details

No response

@r12f r12f added the question the issue author asks something label Jan 15, 2023
@fdncred
Copy link
Collaborator

fdncred commented Jan 15, 2023

This seems to work.

alias ll = ls -l

This may also work.

def lsl [...x] { ls -l ($x | str join) }

@r12f
Copy link
Author

r12f commented Jan 15, 2023

Thank you so much Darren! The alias one works like a charm! but the custom function still has some problem, it cannot pass the flags down as below. Is there a way to work around this? With this, we could implement a bit more complicated aliases, such as running ansible: def asbl-dv [name: string, var_name: string, ...args: string] { ansible $name -e @$env.ANSIBLE_VAULT_FILE -m debug -a "var=$var_name" $args; }

X:\Code〉def lsl [...x] { ls -l ($x | str join) }                                                                                                                                          
X:\Code〉lsl *.zip -a                                                                                                                                                                      01/15/2023 09:23:50 AMError: nu::parser::unknown_flag (link)

  × The `lsl` command doesn't have flag `-a`.
   ╭─[entry #6:1:1]
 1 │ lsl *.zip -a
   ·            ┬
   ·            ╰── unknown flag
   ╰────
  help: Available flags: --help(-h). Use `--help` for more information.

@fdncred
Copy link
Collaborator

fdncred commented Jan 15, 2023

ya, i'm not sure why flags don't work. You can do lsl *.md and it works fine but if you do lsl -a or lsl *.md --all, the flag breaks it.

@r12f
Copy link
Author

r12f commented Jan 15, 2023

yea, my guess is that any flags we use has to be defined as parameter of the custom command and they cannot be captured by rest parameter "...". and this makes implementing pass thru commands really hard....

@fdncred
Copy link
Collaborator

fdncred commented Jan 15, 2023

@kubouch did your pass-through PR address this problem or is this a different one? (or maybe it wasn't yours, i can't remember)

@kubouch
Copy link
Contributor

kubouch commented Jan 16, 2023

Pass-through signatures work only for extern and exec, not custom commands. It's currently not possible to pass along arbitrary flags like that with custom commands. You need to define all the valid flags in the signature.

@r12f
Copy link
Author

r12f commented Jan 17, 2023

Thanks Jakub! Is there any plan on adding this feature? This seems to be quite useful for adding helper functions.

@kubouch
Copy link
Contributor

kubouch commented Jan 17, 2023

Adding it to the signature is not ideal because you'd lose the type checking on the flags, and also there is no clear way to pass them to commands.

One thing that would help is adding a traditional function call syntax, so you could write foo $x --bar as foo($x, bar: true). You'd still be required to specify the complete signature, but this new optional syntax would allow you to pass the flags further without much overhead.

@emk
Copy link

emk commented Apr 3, 2023

Thank you for a really amazing shell!

Another use case where this would be extremely helpful would be something like:

# Call ripgrep with the specified arguments and return the results as JSON.
def rgj [...arg: string] {
  rg --json $arg | from json -o
}

This works fine for positional arguments, but not for keywords. ripgrep has about a hundred flags, and I don't really want to define them all manually. And I can't do anything clever with aliases, because I want that trailing from json -o in there.

This seems like a pretty natural and reasonable thing to want do. In Python, I could do something like:

def rgj(*args, **kwargs):
    return rg(*args, json: true, **kwargs)

def rg(*args, **kwargs):
    print(args, kwargs)

rgj("foo", bar="baz")

# Prints ('foo',) {'json': True, 'bar': 'baz'}

In simple cases, sure, it's probably worth re-declaring and re-documenting all the flags to a command. But I'd really love some simple way to do this. And it's not totally unprecedented, because system commands can already take arbitrary, undocumeted flags.

That said, this would require some new machinery to handle a feature analogous to **kwargs. And that has a lot of design consequences...

@pingiun
Copy link
Contributor

pingiun commented Apr 13, 2023

I think we should have some kind of "accept raw argument list for this function" syntax. This would allow you to inspect the list as you'd like, and pass on the list of arguments to an external command or other script.

My proposal is to add the following syntax (maybe with a different sigil):

def foo [ ..*rest ] {
  echo $"First arg of rest: ($rest.0)"
  # Call the external command with the raw arguments
  ^external $rest
}

So basically the variables are collected in a list of strings, just like with ...arg: string, and you can pass along the arguments to an external call.

I think adding some kind of * in the "collect raw" token could be nice, so I'll start experimenting with this.

One thing that would complicate this feature is mixing normal flags and arguments with this raw args syntax. That is why I think this should not be allowed. You can only accept all the raw args, or let nu parse flags and args, but you can't do both. This also means you cannot define different commands with the same prefix like def foo [ ..*rest] ... def "foo bar" [ ..*rest ]. This will prevent the nushell code getting complicated, and will also make this conceptually easier.

@kubouch
Copy link
Contributor

kubouch commented Apr 13, 2023

The problem with kwargs is that you lose the benefits of our signature system: Type checking, completions, docs, etc. If you plan to use kwargs in the signature, you can just as well use a record. It can also create hard-to-follow APIs We're not very keen on adding this at the moment.

The actual problem here is how to pass flags along, for example:

def some-command [--foo, --bar: string] { ... }

def spam [--foo, --bar: string] {
    some-command # how to pass --foo and --bar?
}

One solution (not the only one) is to allow alternative syntax for command call, similar to classic function call syntax:

def spam [--foo, --bar: string] {
    some-command(foo: $foo, bar: $bar)
}

or shorter, if the passed flag name is same as the expected flag name:

def spam [--foo, --bar: string] {
    some-command($foo, $bar)
}

@pingiun
Copy link
Contributor

pingiun commented Apr 13, 2023

My main use case isn't to use this with nu functions but with external commands. They can accept the $rest list just fine. Currently it just isn't possible to create wrapper scripts for external commands with nu. This is a big use case for shells, so this should also be possible in nu shell.

As for applying an internal function with a list of arguments, I briefly talked about this in another issue: #8751. The idea being you add a lisp-style apply function

@pingiun
Copy link
Contributor

pingiun commented Apr 13, 2023

Just to be clear: it should be possible to forward any flags, not just flags you have defined for your function

@kubouch
Copy link
Contributor

kubouch commented Apr 13, 2023

(Ooops, I realized I commented on this issue before already with the same message.)

That's a good point. If we reduce the problem only on externals, it becomes simpler as externals only need strings. We have an existing concept that's used in extern signatures and exec where anything that's not defined in a signature is a string. For example

> extern spam [--foo] 
> spam --foo --bar

would parse/highlight the --foo flag as a flag and --bar as a "--bar" string. We call it a "fall-through" signature. The big downside of fall-through signatures is that if you make a mistake and type --fooo instead of --foo, it won't be caught but instead passed as a string. But, it should solve the external wrapping problem because everything not present in the signature should be grouped under $rest as strings. Allowing it for def signatures does not seem ideal to me, but one possibility could be to allow extern to have blocks (similar to def) where you could put your own implementation.

I don't understand the apply method you mentioned because to use apply spam $command you'd need to collect $command somehow first which is the problem we're trying to solve here. But maybe I'm misunderstanding it, could you put together some complete example using it?

@pingiun
Copy link
Contributor

pingiun commented Apr 13, 2023

I don't understand the apply method you mentioned because to use apply spam $command you'd need to collect $command somehow first which is the problem we're trying to solve here. But maybe I'm misunderstanding it, could you put together some complete example using it?

Yea it's not really applicable to this issue but I'll give a quick example on how it could be used:

let args = if test {
  ["--foo", "bar"]
} else {
  ["--foo", "baz"]
}
# imagine this has multiple such checks, building up an $args list
# now run the internal-fn with the collected args in subcommand
let args = ["subcommand"] + $args
apply internal-fn $args

It may not be the case that this is possible with the current architecture, as the subcommand would be defined as def "internal-fn subcommand" []. I don't know enough about the nu internals. But the apply command could re-parse that $args list to see if it's correct and then apply it to the given function.

@pingiun
Copy link
Contributor

pingiun commented Apr 13, 2023

I think your idea of having extern commands with blocks is a good syntax by the way. It shows users that it's really only meant for talking with external command, even though in evaluating it would behave the same as a function

@kubouch
Copy link
Contributor

kubouch commented Apr 13, 2023

Yeah, constructing commands like that dynamically is currently not possible, so the apply method might not be viable with our design.

@kubouch
Copy link
Contributor

kubouch commented Apr 13, 2023

But yeah, the extern with a block might be a good solution for the external wrapping problem, striking the balance between full kwargs flexibility, and not giving up on our current signature system.

@theAkito
Copy link

theAkito commented May 5, 2023

This issue is based on the root problem described in #7033.

The root problem is a deeper issue, causing several effects, like the one described in this issue, as already mentioned here.

@MilesCranmer
Copy link

Any updates on this?

In bash I might do something like this:

function wrapped_cmd {
    cmd --arg1=2 "$@" --output=~/.cache
}

which lets me do

wrapped_cmd -a 1 -b 2 file.txt
# => cmd --arg1=2 -a 1 -b 2 file.txt --output=~/.cache

It's really useful for creating more complex aliases, especially if I just want to wrap some external command that I don't know all the current (and future) flags for. Curious if there's something that would let me do this in nushell.

@kubouch
Copy link
Contributor

kubouch commented Apr 20, 2024

You can now use the spread operator:

def lsl [...x] { ls -l ...$x }

@kubouch kubouch closed this as completed Apr 20, 2024
@justbispo
Copy link

justbispo commented May 7, 2024

In case anyone stumbles upon this issue as I did after trying a few suggested solutions, the correct (as of v0.93) way of doing this is using the --wrapped flag.

Only works with external commands.

An example if you'd want to shadow bat:

alias _bat = bat
def --wrapped bat [...rest] {
  _bat -p ...$rest
}
bat -n

It should show the output of both -p and -n applied.

@theAkito
Copy link

theAkito commented May 8, 2024

I just used a combination of applying the ...rest parameter attribute to all the options and the --wrapped option to make all this work. Great!

@kubouch
Copy link
Contributor

kubouch commented May 8, 2024

def --wrapped won't work because the flags are collected as strings, so the -f in ls -la -f will be interpreted as a string, not a flag. If you want to pass around flags as flags, you need to encode them into the signature. It should also be _ls -la in the def body, otherwise you'll get a recursion error.

@justbispo
Copy link

def --wrapped won't work because the flags are collected as strings, so the -f in ls -la -f will be interpreted as a string, not a flag. If you want to pass around flags as flags, you need to encode them into the signature.

Yeah, I'm sorry for my mistake. I used ls as a generic example but ls is a built-in command, so it doesn't work the same as external ones. The example does work for external commands tho. Should've tested the example I gave before posting it . I'll fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question the issue author asks something
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants