Remove required positional arg for some file system commands #11858
Conversation
…eady support rest args
| SyntaxShape::GlobPattern, | ||
| "The file or files you want to remove.", | ||
| ) | ||
| .rest("paths", SyntaxShape::GlobPattern, "The file paths(s) to remove.") |
There was a problem hiding this comment.
Do I read that right that this would now not complain about a missing path?
Maybe still worth raising an error at runtime. (Yes you could not completely freely run rm ...$my_unknown_list)
This would kind of give more peace of mind when running rm bare and you don't see any output.
There was a problem hiding this comment.
Maybe we could throw an error only if the syntax tree has no positional/rest arguments (call.rest_iter().next().is_none())? I.e., only a lone rm would fail:
> rm
Error ...
There was a problem hiding this comment.
That would be a possibility, but somehow my taste develops in the direction that our internal commands should pull less tricks that you can not perform as a Nu library writer.
But maybe your suggestion gets us closer to fulfilling the expectations from #11796
There was a problem hiding this comment.
Yeah, in the future we should consider adding the required rest parameter that I think you mentioned. Otherwise, I think this is the best of both worlds that is currently possible.
There was a problem hiding this comment.
Actually, it looks like the other filesystem commands error if the rest arg is empty. I'll just match their behavior for consistency.
|
@IanManske Nice job on all the PRs this week! |
…#11858) # Description Fixes (most of) nushell#11796. Some filesystem commands have a required positional argument which hinders spreading rest args. This PR removes the required positional arg from `rm`, `open`, and `touch` to be consistent with other filesystem commands that already only have a single rest arg (`mkdir` and `cp`). # User-Facing Changes `rm`, `open`, and `touch` might no longer error when they used to, but otherwise there should be no noticeable changes.
…#11858) Fixes (most of) nushell#11796. Some filesystem commands have a required positional argument which hinders spreading rest args. This PR removes the required positional arg from `rm`, `open`, and `touch` to be consistent with other filesystem commands that already only have a single rest arg (`mkdir` and `cp`). `rm`, `open`, and `touch` might no longer error when they used to, but otherwise there should be no noticeable changes.
Description
Fixes (most of) #11796. Some filesystem commands have a required positional argument which hinders spreading rest args. This PR removes the required positional arg from
rm,open, andtouchto be consistent with other filesystem commands that already only have a single rest arg (mkdirandcp).User-Facing Changes
rm,open, andtouchmight no longer error when they used to, but otherwise there should be no noticeable changes.