holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

open, rm, umv, cp, rm and du: Don't globs if inputs are variables or string interpolation (#11886)

Browse Source 
# Description
This is a follow up to
https://github.com/nushell/nushell/pull/11621#issuecomment-1937484322

Also Fixes: #11838 

## About the code change
It applys the same logic when we pass variables to external commands:


0487e9ffcb/crates/nu-command/src/system/run_external.rs (L162-L170)

That is: if user input dynamic things(like variables, sub-expression, or
string interpolation), it returns a quoted `NuPath`, then user input
won't be globbed
 
# User-Facing Changes
Given two input files: `a*c.txt`, `abc.txt`

* `let f = "a*c.txt"; rm $f` will remove one file: `a*c.txt`. 
~* `let f = "a*c.txt"; rm --glob $f` will remove `a*c.txt` and
`abc.txt`~
* `let f: glob = "a*c.txt"; rm $f` will remove `a*c.txt` and `abc.txt`

## Rules about globbing with *variable*
Given two files: `a*c.txt`, `abc.txt`
| Cmd Type | example | Result |
| ----- | ------------------ | ------ |
| builtin | let f = "a*c.txt"; rm $f | remove `a*c.txt` |
| builtin | let f: glob = "a*c.txt"; rm $f | remove `a*c.txt` and
`abc.txt`
| builtin | let f = "a*c.txt"; rm ($f \| into glob) | remove `a*c.txt`
and `abc.txt`
| custom | def crm [f: glob] { rm $f }; let f = "a*c.txt"; crm $f |
remove `a*c.txt` and `abc.txt`
| custom | def crm [f: glob] { rm ($f \| into string) }; let f =
"a*c.txt"; crm $f | remove `a*c.txt`
| custom | def crm [f: string] { rm $f }; let f = "a*c.txt"; crm $f |
remove `a*c.txt`
| custom | def crm [f: string] { rm $f }; let f = "a*c.txt"; crm ($f \|
into glob) | remove `a*c.txt` and `abc.txt`

In general, if a variable is annotated with `glob` type, nushell will
expand glob pattern. Or else, we need to use `into | glob` to expand
glob pattern

# Tests + Formatting
Done

# After Submitting
I think `str glob-escape` command will be no-longer required. We can
remove it.
This commit is contained in:
Wind
2024-02-23 09:17:09 +08:00
committed by GitHub
parent a2a1c1656f
commit f7d647ac3c
41 changed files with 534 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
crates/nu-protocol/src/eval_base.rs
View File

@ -287,11 +287,7 @@ pub trait Eval {
Expr::GlobPattern(pattern, quoted) => {
// GlobPattern is similar to Filepath
// But we don't want to expand path during eval time, it's required for `nu_engine::glob_from` to run correctly
if *quoted {
Ok(Value::quoted_string(pattern, expr.span))
} else {
Ok(Value::string(pattern, expr.span))
}
Ok(Value::glob(pattern, *quoted, expr.span))
}
Expr::MatchBlock(_) // match blocks are handled by `match`
| Expr::VarDecl(_)

2
crates/nu-protocol/src/syntax_shape.rs
View File

@ -150,7 +150,7 @@ impl SyntaxShape {
SyntaxShape::Float => Type::Float,
SyntaxShape::Filesize => Type::Filesize,
SyntaxShape::FullCellPath => Type::Any,
SyntaxShape::GlobPattern => Type::String,
SyntaxShape::GlobPattern => Type::Glob,
SyntaxShape::Error => Type::Error,
SyntaxShape::ImportPattern => Type::Any,
SyntaxShape::Int => Type::Int,

6
crates/nu-protocol/src/ty.rs
View File

@ -31,6 +31,7 @@ pub enum Type {
Record(Vec<(String, Type)>),
Signature,
String,
Glob,
Table(Vec<(String, Type)>),
}
@ -63,6 +64,8 @@ impl Type {
is_subtype_collection(this, that)
}
(Type::Table(_), Type::List(_)) => true,
(Type::Glob, Type::String) => true,
(Type::String, Type::Glob) => true,
_ => false,
}
}
@ -110,6 +113,7 @@ impl Type {
Type::Binary => SyntaxShape::Binary,
Type::Custom(_) => SyntaxShape::Any,
Type::Signature => SyntaxShape::Signature,
Type::Glob => SyntaxShape::GlobPattern,
}
}
@ -139,6 +143,7 @@ impl Type {
Type::Binary => String::from("binary"),
Type::Custom(_) => String::from("custom"),
Type::Signature => String::from("signature"),
Type::Glob => String::from("glob"),
}
}
}
@ -196,6 +201,7 @@ impl Display for Type {
Type::Binary => write!(f, "binary"),
Type::Custom(custom) => write!(f, "{custom}"),
Type::Signature => write!(f, "signature"),
Type::Glob => write!(f, "glob"),
}
}
}

38
crates/nu-protocol/src/value/from_value.rs
View File

@ -1,6 +1,6 @@
use std::path::PathBuf;
use super::NuPath;
use super::NuGlob;
use crate::ast::{CellPath, PathMember};
use crate::engine::{Block, Closure};
use crate::{Range, Record, ShellError, Spanned, Value};
@ -204,13 +204,23 @@ impl FromValue for Spanned<String> {
}
}
impl FromValue for NuPath {
impl FromValue for NuGlob {
fn from_value(v: Value) -> Result<Self, ShellError> {
// FIXME: we may want to fail a little nicer here
match v {
Value::CellPath { val, .. } => Ok(NuPath::UnQuoted(val.to_string())),
Value::String { val, .. } => Ok(NuPath::UnQuoted(val)),
Value::QuotedString { val, .. } => Ok(NuPath::Quoted(val)),
Value::CellPath { val, .. } => Ok(NuGlob::Expand(val.to_string())),
Value::String { val, .. } => Ok(NuGlob::DoNotExpand(val)),
Value::Glob {
val,
no_expand: quoted,
..
} => {
if quoted {
Ok(NuGlob::DoNotExpand(val))
} else {
Ok(NuGlob::Expand(val))
}
}
v => Err(ShellError::CantConvert {
to_type: "string".into(),
from_type: v.get_type().to_string(),
@ -221,14 +231,24 @@ impl FromValue for NuPath {
}
}
impl FromValue for Spanned<NuPath> {
impl FromValue for Spanned<NuGlob> {
fn from_value(v: Value) -> Result<Self, ShellError> {
let span = v.span();
Ok(Spanned {
item: match v {
Value::CellPath { val, .. } => NuPath::UnQuoted(val.to_string()),
Value::String { val, .. } => NuPath::UnQuoted(val),
Value::QuotedString { val, .. } => NuPath::Quoted(val),
Value::CellPath { val, .. } => NuGlob::Expand(val.to_string()),
Value::String { val, .. } => NuGlob::DoNotExpand(val),
Value::Glob {
val,
no_expand: quoted,
..
} => {
if quoted {
NuGlob::DoNotExpand(val)
} else {
NuGlob::Expand(val)
}
}
v => {
return Err(ShellError::CantConvert {
to_type: "string".into(),

37
crates/nu-protocol/src/value/glob.rs Normal file
View File

@ -0,0 +1,37 @@
use serde::Deserialize;
use std::fmt::Display;
// Introduce this `NuGlob` enum rather than using `Value::Glob` directlry
// So we can handle glob easily without considering too much variant of `Value` enum.
#[derive(Debug, Clone, Deserialize)]
pub enum NuGlob {
/// Don't expand the glob pattern, normally it includes a quoted string(except backtick)
/// And a variable that doesn't annotated with `glob` type
DoNotExpand(String),
/// A glob pattern that is required to expand, it includes bare word
/// And a variable which is annotated with `glob` type
Expand(String),
}
impl NuGlob {
pub fn strip_ansi_string_unlikely(self) -> Self {
match self {
NuGlob::DoNotExpand(s) => NuGlob::DoNotExpand(nu_utils::strip_ansi_string_unlikely(s)),
NuGlob::Expand(s) => NuGlob::Expand(nu_utils::strip_ansi_string_unlikely(s)),
}
}
}
impl AsRef<str> for NuGlob {
fn as_ref(&self) -> &str {
match self {
NuGlob::DoNotExpand(s) | NuGlob::Expand(s) => s,
}
}
}
impl Display for NuGlob {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.as_ref())
}
}

63
crates/nu-protocol/src/value/mod.rs
View File

@ -1,8 +1,8 @@
mod custom_value;
mod from;
mod from_value;
mod glob;
mod lazy_record;
mod path;
mod range;
mod stream;
mod unit;
@ -19,13 +19,13 @@ use chrono_humanize::HumanTime;
pub use custom_value::CustomValue;
use fancy_regex::Regex;
pub use from_value::FromValue;
pub use glob::*;
pub use lazy_record::LazyRecord;
use nu_utils::locale::LOCALE_OVERRIDE_ENV_VAR;
use nu_utils::{
contains_emoji, get_system_locale, locale::get_system_locale_string, IgnoreCaseExt,
};
use num_format::ToFormattedString;
pub use path::*;
pub use range::*;
pub use record::Record;
use serde::{Deserialize, Serialize};
@ -93,8 +93,9 @@ pub enum Value {
// please use .span() instead of matching this span value
internal_span: Span,
},
QuotedString {
Glob {
val: String,
no_expand: bool,
// note: spans are being refactored out of Value
// please use .span() instead of matching this span value
internal_span: Span,
@ -188,8 +189,13 @@ impl Clone for Value {
val: val.clone(),
internal_span: *internal_span,
},
Value::QuotedString { val, internal_span } => Value::QuotedString {
Value::Glob {
val,
no_expand: quoted,
internal_span,
} => Value::Glob {
val: val.clone(),
no_expand: *quoted,
internal_span: *internal_span,
},
Value::Record { val, internal_span } => Value::Record {
@ -721,7 +727,7 @@ impl Value {
| Value::Date { internal_span, .. }
| Value::Range { internal_span, .. }
| Value::String { internal_span, .. }
| Value::QuotedString { internal_span, .. }
| Value::Glob { internal_span, .. }
| Value::Record { internal_span, .. }
| Value::List { internal_span, .. }
| Value::Block { internal_span, .. }
@ -746,7 +752,7 @@ impl Value {
| Value::Date { internal_span, .. }
| Value::Range { internal_span, .. }
| Value::String { internal_span, .. }
| Value::QuotedString { internal_span, .. }
| Value::Glob { internal_span, .. }
| Value::Record { internal_span, .. }
| Value::LazyRecord { internal_span, .. }
| Value::List { internal_span, .. }
@ -773,7 +779,7 @@ impl Value {
Value::Date { .. } => Type::Date,
Value::Range { .. } => Type::Range,
Value::String { .. } => Type::String,
Value::QuotedString { .. } => Type::String,
Value::Glob { .. } => Type::Glob,
Value::Record { val, .. } => {
Type::Record(val.iter().map(|(x, y)| (x.clone(), y.get_type())).collect())
}
@ -903,7 +909,7 @@ impl Value {
)
}
Value::String { val, .. } => val.clone(),
Value::QuotedString { val, .. } => val.clone(),
Value::Glob { val, .. } => val.clone(),
Value::List { vals: val, .. } => format!(
"[{}]",
val.iter()
@ -1891,9 +1897,10 @@ impl Value {
}
}
pub fn quoted_string(val: impl Into<String>, span: Span) -> Value {
Value::QuotedString {
pub fn glob(val: impl Into<String>, no_expand: bool, span: Span) -> Value {
Value::Glob {
val: val.into(),
no_expand,
internal_span: span,
}
}
@ -2144,7 +2151,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2165,7 +2172,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2186,7 +2193,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2207,7 +2214,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2228,7 +2235,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2249,7 +2256,7 @@ impl PartialOrd for Value {
Value::Date { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2270,7 +2277,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2291,7 +2298,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::QuotedString { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2303,7 +2310,7 @@ impl PartialOrd for Value {
Value::CellPath { .. } => Some(Ordering::Less),
Value::CustomValue { .. } => Some(Ordering::Less),
},
(Value::QuotedString { val: lhs, .. }, rhs) => match rhs {
(Value::Glob { val: lhs, .. }, rhs) => match rhs {
Value::Bool { .. } => Some(Ordering::Greater),
Value::Int { .. } => Some(Ordering::Greater),
Value::Float { .. } => Some(Ordering::Greater),
@ -2312,7 +2319,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::QuotedString { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2333,7 +2340,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { val: rhs, .. } => {
// reorder cols and vals to make more logically compare.
// more general, if two record have same col and values,
@ -2373,7 +2380,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { vals: rhs, .. } => lhs.partial_cmp(rhs),
@ -2394,7 +2401,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
@ -2415,7 +2422,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2436,7 +2443,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2457,7 +2464,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2478,7 +2485,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2499,7 +2506,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),

22
crates/nu-protocol/src/value/path.rs
View File

@ -1,36 +1,32 @@
use serde::Deserialize;
use std::fmt::Display;
/// A simple wrapper to String.
///
/// But it tracks if the string is originally quoted.
/// So commands can make decision on path auto-expanding behavior.
#[derive(Debug, Clone, Deserialize)]
pub enum NuPath {
pub enum NuGlob {
/// A quoted path(except backtick), in this case, nushell shouldn't auto-expand path.
Quoted(String),
NoExpand(String),
/// An unquoted path, in this case, nushell should auto-expand path.
UnQuoted(String),
NeedExpand(String),
}
impl NuPath {
impl NuGlob {
pub fn strip_ansi_string_unlikely(self) -> Self {
match self {
NuPath::Quoted(s) => NuPath::Quoted(nu_utils::strip_ansi_string_unlikely(s)),
NuPath::UnQuoted(s) => NuPath::UnQuoted(nu_utils::strip_ansi_string_unlikely(s)),
NuGlob::NoExpand(s) => NuGlob::NoExpand(nu_utils::strip_ansi_string_unlikely(s)),
NuGlob::NeedExpand(s) => NuGlob::NeedExpand(nu_utils::strip_ansi_string_unlikely(s)),
}
}
}
impl AsRef<str> for NuPath {
impl AsRef<str> for NuGlob {
fn as_ref(&self) -> &str {
match self {
NuPath::Quoted(s) | NuPath::UnQuoted(s) => s,
NuGlob::NoExpand(s) | NuGlob::NeedExpand(s) => s,
}
}
}
impl Display for NuPath {
impl Display for NuGlob {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.as_ref())
}