do not attempt to glob expand if the file path is wrapped in quotes#11569
do not attempt to glob expand if the file path is wrapped in quotes#11569WindSoilder merged 8 commits intonushell:mainfrom
Conversation
|
I think this is a creative solution @WindSoilder. Nice work. I have a couple questions.
|
The changes on |
|
I'm thinking that do we need to apply the same logic on cp, mv, rm. Which is: if quoted, get rid of globbing. If not quoted, apply the glob operation. If we do so, nushell will get better consistency on globbing rules across different fs commands. But it belongs to another problem. |
|
What i'm suggesting then is 2/bob's pr should be reverted since it has extra overhead to check things twice (in some cases). |
|
Yeah, that's a good point. Maybe I can try to handle it in another pr? |
|
ya, i'd be fine with a separate pr for that. |
|
I'll merge it if there is no objections, any thought? |
|
no objections from me. i think it's worth a try. |
|
OK, thanks for your input, you guide me to a good direction to improve some other commands |
…`du` commands (#11621) # Description This pr is a follow up to [#11569](#11569 (comment)) > Revert the logic in #10694 and apply the logic in this pr to mv, cp, rv will require a larger change, I need to think how to achieve the bahavior And sorry @bobhy for reverting some of your changes. This pr is going to unify glob behavior on the given commands: * open * rm * cp-old * mv * umv * cp * du So they have the same behavior to `ls`, which is: If given parameter is quoted by single quote(`'`) or double quote(`"`), don't auto-expand the glob pattern. If not quoted, auto-expand the glob pattern. Fixes: #9558 Fixes: #10211 Fixes: #9310 Fixes: #10364 # TODO But there is one thing remains: if we give a variable to the command, it will always auto-expand the glob pattern, e.g: ```nushell let path = "a[123]b" rm $path ``` I don't think it's expected. But I also think user might want to auto-expand the glob pattern in variables. So I'll introduce a new command called `glob escape`, then if user doesn't want to auto-expand the glob pattern, he can just do this: `rm ($path | glob escape)` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting Done # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## NOTE This pr changes the semantic of `GlobPattern`, before this pr, it will `expand path` after evaluated, this makes `nu_engine::glob_from` have no chance to glob things right if a path contains glob pattern. e.g: [#9310 ](#9310 (comment)) #10211 I think changing the semantic is fine, because it makes glob works if path contains something like '*'. It maybe a breaking change if a custom command's argument are annotated by `: glob`.
|
After this PR, how are we supposed to deal with globs containing spaces? in POSIX shells one can just escape the space directly, but that's not the case in nushell |
|
@crides do backticks work? they seem to work for me |
|
oh okay, backticks are not included. Thanks |
…ushell#11569) # Description Fixes: nushell#11455 ### For arguments which is annotated with `:path/:directory/:glob` To fix the issue, we need to have a way to know if a path is originally quoted during runtime. So the information needed to be added at several levels: * parse time (from user input to expression) We need to add quoted information into `Expr::Filepath`, `Expr::Directory`, `Expr::GlobPattern` * eval time When convert from `Expr::Filepath`, `Expr::Directory`, `Expr::GlobPattern` to `Value::String` during runtime, we won't auto expanded the path if it's quoted ### For `ls` It's really special, because it accepts a `String` as a pattern, and it generates `glob` expression inside the command itself. So the idea behind the change is introducing a special SyntaxShape to ls: `SyntaxShape::LsGlobPattern`. So we can track if the pattern is originally quoted easier, and we don't auto expand the path either. Then when constructing a glob pattern inside ls, we check if input pattern is quoted, if so: we escape the input pattern, so we can run `ls a[123]b`, because it's already escaped. Finally, to accomplish the checking process, we also need to introduce a new value type called `Value::QuotedString` to differ from `Value::String`, it's used to generate an enum called `NuPath`, which is finally used in `ls` function. `ls` learned from `NuPath` to know if user input is quoted. # User-Facing Changes Actually it contains several changes ### For arguments which is annotated with `:path/:directory/:glob` #### Before ```nushell > def foo [p: path] { echo $p }; print (foo "~/a"); print (foo '~/a') /home/windsoilder/a /home/windsoilder/a > def foo [p: directory] { echo $p }; print (foo "~/a"); print (foo '~/a') /home/windsoilder/a /home/windsoilder/a > def foo [p: glob] { echo $p }; print (foo "~/a"); print (foo '~/a') /home/windsoilder/a /home/windsoilder/a ``` #### After ```nushell > def foo [p: path] { echo $p }; print (foo "~/a"); print (foo '~/a') ~/a ~/a > def foo [p: directory] { echo $p }; print (foo "~/a"); print (foo '~/a') ~/a ~/a > def foo [p: glob] { echo $p }; print (foo "~/a"); print (foo '~/a') ~/a ~/a ``` ### For ls command `touch '[uwu]'` #### Before ``` ❯ ls -D "[uwu]" Error: × No matches found for [uwu] ╭─[entry nushell#6:1:1] 1 │ ls -D "[uwu]" · ───┬─── · ╰── Pattern, file or folder not found ╰──── help: no matches found ``` #### After ``` ❯ ls -D "[uwu]" ╭───┬───────┬──────┬──────┬──────────╮ │ # │ name │ type │ size │ modified │ ├───┼───────┼──────┼──────┼──────────┤ │ 0 │ [uwu] │ file │ 0 B │ now │ ╰───┴───────┴──────┴──────┴──────────╯ ``` # Tests + Formatting Done # After Submitting NaN
…`du` commands (nushell#11621) # Description This pr is a follow up to [nushell#11569](nushell#11569 (comment)) > Revert the logic in nushell#10694 and apply the logic in this pr to mv, cp, rv will require a larger change, I need to think how to achieve the bahavior And sorry @bobhy for reverting some of your changes. This pr is going to unify glob behavior on the given commands: * open * rm * cp-old * mv * umv * cp * du So they have the same behavior to `ls`, which is: If given parameter is quoted by single quote(`'`) or double quote(`"`), don't auto-expand the glob pattern. If not quoted, auto-expand the glob pattern. Fixes: nushell#9558 Fixes: nushell#10211 Fixes: nushell#9310 Fixes: nushell#10364 # TODO But there is one thing remains: if we give a variable to the command, it will always auto-expand the glob pattern, e.g: ```nushell let path = "a[123]b" rm $path ``` I don't think it's expected. But I also think user might want to auto-expand the glob pattern in variables. So I'll introduce a new command called `glob escape`, then if user doesn't want to auto-expand the glob pattern, he can just do this: `rm ($path | glob escape)` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting Done # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## NOTE This pr changes the semantic of `GlobPattern`, before this pr, it will `expand path` after evaluated, this makes `nu_engine::glob_from` have no chance to glob things right if a path contains glob pattern. e.g: [nushell#9310 ](nushell#9310 (comment)) nushell#10211 I think changing the semantic is fine, because it makes glob works if path contains something like '*'. It maybe a breaking change if a custom command's argument are annotated by `: glob`.
Description
Fixes: #11455
For arguments which is annotated with
:path/:directory/:globTo fix the issue, we need to have a way to know if a path is originally quoted during runtime. So the information needed to be added at several levels:
We need to add quoted information into
Expr::Filepath,Expr::Directory,Expr::GlobPatternWhen convert from
Expr::Filepath,Expr::Directory,Expr::GlobPatterntoValue::Stringduring runtime, we won't auto expanded the path if it's quotedFor
lsIt's really special, because it accepts a
Stringas a pattern, and it generatesglobexpression inside the command itself.So the idea behind the change is introducing a special SyntaxShape to ls:
SyntaxShape::LsGlobPattern. So we can track if the pattern is originally quoted easier, and we don't auto expand the path either.Then when constructing a glob pattern inside ls, we check if input pattern is quoted, if so: we escape the input pattern, so we can run
ls a[123]b, because it's already escaped.Finally, to accomplish the checking process, we also need to introduce a new value type called
Value::QuotedStringto differ fromValue::String, it's used to generate an enum calledNuPath, which is finally used inlsfunction.lslearned fromNuPathto know if user input is quoted.User-Facing Changes
Actually it contains several changes
For arguments which is annotated with
:path/:directory/:globBefore
After
For ls command
touch '[uwu]'Before
After
Tests + Formatting
Done
After Submitting
NaN