Skip to content

Commit

Permalink
open, rm, umv, cp, rm and du: Don't globs if inputs are v…
Browse files Browse the repository at this point in the history
…ariables or string interpolation (#11886)

# Description
This is a follow up to
nushell/nushell#11621 (comment)

Also Fixes: #11838 

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


https://github.com/nushell/nushell/blob/0487e9ffcbc57c2d5feca606e10c3f8221ff5e00/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.
  • Loading branch information
WindSoilder committed Feb 23, 2024
1 parent a2a1c16 commit f7d647a
Show file tree
Hide file tree
Showing 41 changed files with 534 additions and 109 deletions.
2 changes: 1 addition & 1 deletion crates/nu-cmd-lang/src/core_commands/describe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ fn describe_value(
| Value::Date { .. }
| Value::Range { .. }
| Value::String { .. }
| Value::QuotedString { .. }
| Value::Glob { .. }
| Value::Nothing { .. } => Value::record(
record!(
"type" => Value::string(value.get_type().to_string(), head),
Expand Down
22 changes: 20 additions & 2 deletions crates/nu-cmd-lang/src/core_commands/let_.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use nu_engine::eval_block;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, SyntaxShape, Type};
use nu_protocol::{
Category, Example, PipelineData, ShellError, Signature, SyntaxShape, Type, Value,
};

#[derive(Clone)]
pub struct Let;
Expand Down Expand Up @@ -61,8 +63,24 @@ impl Command for Let {
.expect("internal error: missing right hand side");

let block = engine_state.get_block(block_id);

let pipeline_data = eval_block(engine_state, stack, block, input, true, false)?;
stack.add_var(var_id, pipeline_data.into_value(call.head));
let mut value = pipeline_data.into_value(call.head);

// if given variable type is Glob, and our result is string
// then nushell need to convert from Value::String to Value::Glob
// it's assigned by demand, then it's not quoted, and it's required to expand
// if we pass it to other commands.
let var_type = &engine_state.get_var(var_id).ty;
let val_span = value.span();
match value {
Value::String { val, .. } if var_type == &Type::Glob => {
value = Value::glob(val, false, val_span);
}
_ => {}
}

stack.add_var(var_id, value);
Ok(PipelineData::empty())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cmd-lang/src/example_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl<'a> std::fmt::Debug for DebuggableValue<'a> {
val.from, val.to, val.incr
),
},
Value::String { val, .. } | Value::QuotedString { val, .. } => {
Value::String { val, .. } | Value::Glob { val, .. } => {
write!(f, "{:?}", val)
}
Value::Record { val, .. } => {
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-color-config/src/style_computer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl<'a> StyleComputer<'a> {
Value::Range { .. } => TextStyle::with_style(Left, s),
Value::Float { .. } => TextStyle::with_style(Right, s),
Value::String { .. } => TextStyle::with_style(Left, s),
Value::QuotedString { .. } => TextStyle::with_style(Left, s),
Value::Glob { .. } => TextStyle::with_style(Left, s),
Value::Nothing { .. } => TextStyle::with_style(Left, s),
Value::Binary { .. } => TextStyle::with_style(Left, s),
Value::CellPath { .. } => TextStyle::with_style(Left, s),
Expand Down
133 changes: 133 additions & 0 deletions crates/nu-command/src/conversions/into/glob.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
use nu_cmd_base::input_handler::{operate, CmdArgument};
use nu_engine::CallExt;
use nu_protocol::{
ast::{Call, CellPath},
engine::{Command, EngineState, Stack},
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, SyntaxShape,
Type, Value,
};

struct Arguments {
cell_paths: Option<Vec<CellPath>>,
}

impl CmdArgument for Arguments {
fn take_cell_paths(&mut self) -> Option<Vec<CellPath>> {
self.cell_paths.take()
}
}

#[derive(Clone)]
pub struct SubCommand;

impl Command for SubCommand {
fn name(&self) -> &str {
"into glob"
}

fn signature(&self) -> Signature {
Signature::build("into glob")
.input_output_types(vec![
(Type::String, Type::Glob),
(
Type::List(Box::new(Type::String)),
Type::List(Box::new(Type::Glob)),
),
(Type::Table(vec![]), Type::Table(vec![])),
(Type::Record(vec![]), Type::Record(vec![])),
])
.allow_variants_without_examples(true) // https://github.com/nushell/nushell/issues/7032
.rest(
"rest",
SyntaxShape::CellPath,
"For a data structure input, convert data at the given cell paths.",
)
.category(Category::Conversions)
}

fn usage(&self) -> &str {
"Convert value to glob."
}

fn search_terms(&self) -> Vec<&str> {
vec!["convert", "text"]
}

fn run(
&self,
engine_state: &EngineState,
stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
glob_helper(engine_state, stack, call, input)
}

fn examples(&self) -> Vec<Example> {
vec![
Example {
description: "convert string to glob",
example: "'1234' | into glob",
result: Some(Value::test_string("1234")),
},
Example {
description: "convert filepath to string",
example: "ls Cargo.toml | get name | into glob",
result: None,
},
]
}
}

fn glob_helper(
engine_state: &EngineState,
stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let head = call.head;
let cell_paths = call.rest(engine_state, stack, 0)?;
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
let args = Arguments { cell_paths };
match input {
PipelineData::ExternalStream { stdout: None, .. } => {
Ok(Value::glob(String::new(), false, head).into_pipeline_data())
}
PipelineData::ExternalStream {
stdout: Some(stream),
..
} => {
// TODO: in the future, we may want this to stream out, converting each to bytes
let output = stream.into_string()?;
Ok(Value::glob(output.item, false, head).into_pipeline_data())
}
_ => operate(action, args, input, head, engine_state.ctrlc.clone()),
}
}

fn action(input: &Value, _args: &Arguments, span: Span) -> Value {
match input {
Value::String { val, .. } => Value::glob(val.to_string(), false, span),
x => Value::error(
ShellError::CantConvert {
to_type: String::from("glob"),
from_type: x.get_type().to_string(),
span,
help: None,
},
span,
),
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_examples() {
use crate::test_examples;

test_examples(SubCommand {})
}
}
2 changes: 2 additions & 0 deletions crates/nu-command/src/conversions/into/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod datetime;
mod duration;
mod filesize;
mod float;
mod glob;
mod int;
mod record;
mod string;
Expand All @@ -19,6 +20,7 @@ pub use command::Into;
pub use datetime::SubCommand as IntoDatetime;
pub use duration::SubCommand as IntoDuration;
pub use float::SubCommand as IntoFloat;
pub use glob::SubCommand as IntoGlob;
pub use int::SubCommand as IntoInt;
pub use record::SubCommand as IntoRecord;
pub use string::SubCommand as IntoString;
Expand Down
2 changes: 2 additions & 0 deletions crates/nu-command/src/conversions/into/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ impl Command for SubCommand {
(Type::Int, Type::String),
(Type::Number, Type::String),
(Type::String, Type::String),
(Type::Glob, Type::String),
(Type::Bool, Type::String),
(Type::Filesize, Type::String),
(Type::Date, Type::String),
Expand Down Expand Up @@ -202,6 +203,7 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
Value::Bool { val, .. } => Value::string(val.to_string(), span),
Value::Date { val, .. } => Value::string(val.format("%c").to_string(), span),
Value::String { val, .. } => Value::string(val.to_string(), span),
Value::Glob { val, .. } => Value::string(val.to_string(), span),

Value::Filesize { val: _, .. } => {
Value::string(input.to_expanded_string(", ", config), span)
Expand Down
1 change: 1 addition & 0 deletions crates/nu-command/src/database/commands/into_sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ fn nu_value_to_sqlite_type(val: &Value) -> Result<&'static str, ShellError> {
| Type::Range
| Type::Record(_)
| Type::Signature
| Type::Glob
| Type::Table(_) => Err(ShellError::OnlySupportsThisInputType {
exp_input_type: "sql".into(),
wrong_type: val.get_type().to_string(),
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/debug/explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ pub fn debug_string_without_formatting(value: &Value) -> String {
)
}
Value::String { val, .. } => val.clone(),
Value::QuotedString { val, .. } => val.clone(),
Value::Glob { val, .. } => val.clone(),
Value::List { vals: val, .. } => format!(
"[{}]",
val.iter()
Expand Down
1 change: 1 addition & 0 deletions crates/nu-command/src/default_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ pub fn add_shell_command_context(mut engine_state: EngineState) -> EngineState {
IntoInt,
IntoRecord,
IntoString,
IntoGlob,
IntoValue,
};

Expand Down
11 changes: 6 additions & 5 deletions crates/nu-command/src/filesystem/du.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use super::util::opt_for_glob_pattern;
use crate::{DirBuilder, DirInfo, FileInfo};
use nu_engine::{current_dir, CallExt};
use nu_glob::Pattern;
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
Category, Example, IntoInterruptiblePipelineData, NuPath, PipelineData, ShellError, Signature,
Category, Example, IntoInterruptiblePipelineData, NuGlob, PipelineData, ShellError, Signature,
Span, Spanned, SyntaxShape, Type, Value,
};
use serde::Deserialize;
Expand All @@ -14,7 +15,7 @@ pub struct Du;

#[derive(Deserialize, Clone, Debug)]
pub struct DuArgs {
path: Option<Spanned<NuPath>>,
path: Option<Spanned<NuGlob>>,
all: bool,
deref: bool,
exclude: Option<Spanned<String>>,
Expand Down Expand Up @@ -66,7 +67,7 @@ impl Command for Du {
"Exclude files below this size",
Some('m'),
)
.category(Category::Core)
.category(Category::FileSystem)
}

fn run(
Expand Down Expand Up @@ -96,7 +97,7 @@ impl Command for Du {
let current_dir = current_dir(engine_state, stack)?;

let args = DuArgs {
path: call.opt(engine_state, stack, 0)?,
path: opt_for_glob_pattern(engine_state, stack, call, 0)?,
all: call.has_flag(engine_state, stack, "all")?,
deref: call.has_flag(engine_state, stack, "deref")?,
exclude: call.get_flag(engine_state, stack, "exclude")?,
Expand All @@ -119,7 +120,7 @@ impl Command for Du {
// The * pattern should never fail.
None => nu_engine::glob_from(
&Spanned {
item: NuPath::UnQuoted("*".into()),
item: NuGlob::Expand("*".into()),
span: Span::unknown(),
},
&current_dir,
Expand Down
20 changes: 10 additions & 10 deletions crates/nu-command/src/filesystem/ls.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::util::opt_for_glob_pattern;
use crate::DirBuilder;
use crate::DirInfo;
use chrono::{DateTime, Local, LocalResult, TimeZone, Utc};
Expand All @@ -7,7 +8,7 @@ use nu_glob::{MatchOptions, Pattern};
use nu_path::expand_to_real_path;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::NuPath;
use nu_protocol::NuGlob;
use nu_protocol::{
Category, DataSource, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData,
PipelineMetadata, Record, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
Expand Down Expand Up @@ -86,17 +87,16 @@ impl Command for Ls {
let call_span = call.head;
let cwd = current_dir(engine_state, stack)?;

let pattern_arg: Option<Spanned<NuPath>> = call.opt(engine_state, stack, 0)?;

let pattern_arg = opt_for_glob_pattern(engine_state, stack, call, 0)?;
let pattern_arg = {
if let Some(path) = pattern_arg {
match path.item {
NuPath::Quoted(p) => Some(Spanned {
item: NuPath::Quoted(nu_utils::strip_ansi_string_unlikely(p)),
NuGlob::DoNotExpand(p) => Some(Spanned {
item: NuGlob::DoNotExpand(nu_utils::strip_ansi_string_unlikely(p)),
span: path.span,
}),
NuPath::UnQuoted(p) => Some(Spanned {
item: NuPath::UnQuoted(nu_utils::strip_ansi_string_unlikely(p)),
NuGlob::Expand(p) => Some(Spanned {
item: NuGlob::Expand(nu_utils::strip_ansi_string_unlikely(p)),
span: path.span,
}),
}
Expand Down Expand Up @@ -149,7 +149,7 @@ impl Command for Ls {
p,
p_tag,
absolute_path,
matches!(pat.item, NuPath::Quoted(_)),
matches!(pat.item, NuGlob::DoNotExpand(_)),
)
}
None => {
Expand Down Expand Up @@ -186,8 +186,8 @@ impl Command for Ls {
};

let glob_path = Spanned {
// It needs to be un-quoted, the relative logic is handled previously
item: NuPath::UnQuoted(path.clone()),
// use NeedExpand, the relative escaping logic is handled previously
item: NuGlob::Expand(path.clone()),
span: p_tag,
};

Expand Down
4 changes: 2 additions & 2 deletions crates/nu-command/src/filesystem/mv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use nu_engine::CallExt;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Example, IntoInterruptiblePipelineData, NuPath, PipelineData, ShellError, Signature,
Category, Example, IntoInterruptiblePipelineData, NuGlob, PipelineData, ShellError, Signature,
Span, Spanned, SyntaxShape, Type, Value,
};

Expand Down Expand Up @@ -62,7 +62,7 @@ impl Command for Mv {
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
// TODO: handle invalid directory or insufficient permissions when moving
let mut spanned_source: Spanned<NuPath> = call.req(engine_state, stack, 0)?;
let mut spanned_source: Spanned<NuGlob> = call.req(engine_state, stack, 0)?;
spanned_source.item = spanned_source.item.strip_ansi_string_unlikely();
let spanned_destination: Spanned<String> = call.req(engine_state, stack, 1)?;
let verbose = call.has_flag(engine_state, stack, "verbose")?;
Expand Down
Loading

0 comments on commit f7d647a

Please sign in to comment.