Remove required positional arg for some file system commands (#11858)

# 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`, 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.
This commit is contained in:
Ian Manske 2024-02-15 17:25:50 +00:00 committed by GitHub
parent 317653d5d2
commit 903afda6d9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 39 additions and 58 deletions

View File

@ -5,7 +5,7 @@ use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::util::BufferedReader;
use nu_protocol::{
Category, DataSource, Example, IntoInterruptiblePipelineData, NuPath, PipelineData,
PipelineMetadata, RawStream, ShellError, Signature, Spanned, SyntaxShape, Type, Value,
PipelineMetadata, RawStream, ShellError, Signature, Spanned, SyntaxShape, Type,
};
use std::io::BufReader;
@ -42,12 +42,7 @@ impl Command for Open {
fn signature(&self) -> nu_protocol::Signature {
Signature::build("open")
.input_output_types(vec![(Type::Nothing, Type::Any), (Type::String, Type::Any)])
.optional("filename", SyntaxShape::GlobPattern, "The filename to use.")
.rest(
"filenames",
SyntaxShape::GlobPattern,
"Optional additional files to open.",
)
.rest("files", SyntaxShape::GlobPattern, "The file(s) to open.")
.switch("raw", "open file as raw binary", Some('r'))
.category(Category::FileSystem)
}
@ -63,21 +58,11 @@ impl Command for Open {
let call_span = call.head;
let ctrlc = engine_state.ctrlc.clone();
let cwd = current_dir(engine_state, stack)?;
let req_path = call.opt::<Spanned<NuPath>>(engine_state, stack, 0)?;
let mut path_params = call.rest::<Spanned<NuPath>>(engine_state, stack, 1)?;
let mut paths = call.rest::<Spanned<NuPath>>(engine_state, stack, 0)?;
// FIXME: JT: what is this doing here?
if let Some(filename) = req_path {
path_params.insert(0, filename);
} else {
if paths.is_empty() && call.rest_iter(0).next().is_none() {
// try to use path from pipeline input if there were no positional or spread args
let filename = match input {
PipelineData::Value(Value::Nothing { .. }, ..) => {
return Err(ShellError::MissingParameter {
param_name: "needs filename".to_string(),
span: call.head,
})
}
PipelineData::Value(val, ..) => val.as_spanned_string()?,
_ => {
return Err(ShellError::MissingParameter {
@ -87,18 +72,15 @@ impl Command for Open {
}
};
path_params.insert(
0,
Spanned {
item: NuPath::UnQuoted(filename.item),
span: filename.span,
},
);
paths.push(Spanned {
item: NuPath::UnQuoted(filename.item),
span: filename.span,
});
}
let mut output = vec![];
for mut path in path_params.into_iter() {
for mut path in paths {
//FIXME: `open` should not have to do this
path.item = path.item.strip_ansi_string_unlikely();

View File

@ -40,13 +40,9 @@ impl Command for Rm {
}
fn signature(&self) -> Signature {
let sig = Signature::build("rm")
Signature::build("rm")
.input_output_types(vec![(Type::Nothing, Type::Nothing)])
.required(
"filename",
SyntaxShape::GlobPattern,
"The file or files you want to remove.",
)
.rest("paths", SyntaxShape::GlobPattern, "The file paths(s) to remove.")
.switch(
"trash",
"move to the platform's trash instead of permanently deleting. not used on android and ios",
@ -56,8 +52,8 @@ impl Command for Rm {
"permanent",
"delete permanently, ignoring the 'always_trash' config option. always enabled on android and ios",
Some('p'),
);
sig.switch("recursive", "delete subdirectories recursively", Some('r'))
)
.switch("recursive", "delete subdirectories recursively", Some('r'))
.switch("force", "suppress error when no file", Some('f'))
.switch("verbose", "print names of deleted files", Some('v'))
.switch("interactive", "ask user to confirm action", Some('i'))
@ -66,11 +62,6 @@ impl Command for Rm {
"ask user to confirm action only once",
Some('I'),
)
.rest(
"rest",
SyntaxShape::GlobPattern,
"Additional file path(s) to remove.",
)
.category(Category::FileSystem)
}
@ -135,7 +126,14 @@ fn rm(
let ctrlc = engine_state.ctrlc.clone();
let mut targets: Vec<Spanned<NuPath>> = call.rest(engine_state, stack, 0)?;
let mut paths: Vec<Spanned<NuPath>> = call.rest(engine_state, stack, 0)?;
if paths.is_empty() {
return Err(ShellError::MissingParameter {
param_name: "requires file paths".to_string(),
span: call.head,
});
}
let mut unique_argument_check = None;
@ -156,7 +154,7 @@ fn rm(
.into()
});
for (idx, path) in targets.clone().into_iter().enumerate() {
for (idx, path) in paths.clone().into_iter().enumerate() {
if let Some(ref home) = home {
if expand_path_with(path.item.as_ref(), &currentdir_path)
.to_string_lossy()
@ -173,7 +171,7 @@ fn rm(
},
span: path.span,
};
let _ = std::mem::replace(&mut targets[idx], corrected_path);
let _ = std::mem::replace(&mut paths[idx], corrected_path);
}
let span = call.head;
@ -204,7 +202,7 @@ fn rm(
}
}
if targets.is_empty() {
if paths.is_empty() {
return Err(ShellError::GenericError {
error: "rm requires target paths".into(),
msg: "needs parameter".into(),
@ -225,12 +223,12 @@ fn rm(
}
let targets_span = Span::new(
targets
paths
.iter()
.map(|x| x.span.start)
.min()
.expect("targets were empty"),
targets
paths
.iter()
.map(|x| x.span.end)
.max()
@ -240,7 +238,7 @@ fn rm(
let (mut target_exists, mut empty_span) = (false, call.head);
let mut all_targets: HashMap<PathBuf, Span> = HashMap::new();
for target in targets {
for target in paths {
let path = expand_path_with(target.item.as_ref(), &currentdir_path);
if currentdir_path.to_string_lossy() == path.to_string_lossy()
|| currentdir_path.starts_with(format!("{}{}", target.item, std::path::MAIN_SEPARATOR))

View File

@ -25,12 +25,8 @@ impl Command for Touch {
fn signature(&self) -> Signature {
Signature::build("touch")
.input_output_types(vec![ (Type::Nothing, Type::Nothing) ])
.required(
"filename",
SyntaxShape::Filepath,
"The path of the file you want to create.",
)
.input_output_types(vec![(Type::Nothing, Type::Nothing)])
.rest("files", SyntaxShape::Filepath, "The file(s) to create.")
.named(
"reference",
SyntaxShape::String,
@ -52,7 +48,6 @@ impl Command for Touch {
"do not create the file if it does not exist",
Some('c'),
)
.rest("rest", SyntaxShape::Filepath, "Additional files to create.")
.category(Category::FileSystem)
}
@ -71,8 +66,14 @@ impl Command for Touch {
let mut change_atime: bool = call.has_flag(engine_state, stack, "access")?;
let use_reference: bool = call.has_flag(engine_state, stack, "reference")?;
let no_create: bool = call.has_flag(engine_state, stack, "no-create")?;
let target: String = call.req(engine_state, stack, 0)?;
let rest: Vec<String> = call.rest(engine_state, stack, 1)?;
let files: Vec<String> = call.rest(engine_state, stack, 0)?;
if files.is_empty() {
return Err(ShellError::MissingParameter {
param_name: "requires file paths".to_string(),
span: call.head,
});
}
let mut date: Option<DateTime<Local>> = None;
let mut ref_date_atime: Option<DateTime<Local>> = None;
@ -127,7 +128,7 @@ impl Command for Touch {
}
}
for (index, item) in vec![target].into_iter().chain(rest).enumerate() {
for (index, item) in files.into_iter().enumerate() {
if no_create {
let path = Path::new(&item);
if !path.exists() {