Refactor message printing in rm (#12799)

# Description
Changes the iterator in `rm` to be an iterator over
`Result<Option<String>, ShellError>` (an optional message or error)
instead of an iterator over `Value`. Then, the iterator is consumed and
each message is printed. This allows the
`PipelineData::print_not_formatted` method to be removed.
This commit is contained in:
Ian Manske 2024-05-09 05:36:47 +00:00 committed by GitHub
parent 948b299e65
commit 3b3f48202c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 116 additions and 146 deletions

View file

@ -3,7 +3,7 @@ use super::util::{get_rest_for_glob_pattern, try_interaction};
use nu_engine::{command_prelude::*, env::current_dir}; use nu_engine::{command_prelude::*, env::current_dir};
use nu_glob::MatchOptions; use nu_glob::MatchOptions;
use nu_path::expand_path_with; use nu_path::expand_path_with;
use nu_protocol::NuGlob; use nu_protocol::{report_error_new, NuGlob};
#[cfg(unix)] #[cfg(unix)]
use std::os::unix::prelude::FileTypeExt; use std::os::unix::prelude::FileTypeExt;
use std::{ use std::{
@ -118,8 +118,6 @@ fn rm(
let interactive = call.has_flag(engine_state, stack, "interactive")?; let interactive = call.has_flag(engine_state, stack, "interactive")?;
let interactive_once = call.has_flag(engine_state, stack, "interactive-once")? && !interactive; let interactive_once = call.has_flag(engine_state, stack, "interactive-once")? && !interactive;
let ctrlc = engine_state.ctrlc.clone();
let mut paths = get_rest_for_glob_pattern(engine_state, stack, call, 0)?; let mut paths = get_rest_for_glob_pattern(engine_state, stack, call, 0)?;
if paths.is_empty() { if paths.is_empty() {
@ -341,9 +339,7 @@ fn rm(
} }
} }
all_targets let iter = all_targets.into_iter().map(move |(f, span)| {
.into_iter()
.map(move |(f, span)| {
let is_empty = || match f.read_dir() { let is_empty = || match f.read_dir() {
Ok(mut p) => p.next().is_none(), Ok(mut p) => p.next().is_none(),
Err(_) => false, Err(_) => false,
@ -373,8 +369,7 @@ fn rm(
); );
let result = if let Err(e) = interaction { let result = if let Err(e) = interaction {
let e = Error::new(ErrorKind::Other, &*e.to_string()); Err(Error::new(ErrorKind::Other, &*e.to_string()))
Err(e)
} else if interactive && !confirmed { } else if interactive && !confirmed {
Ok(()) Ok(())
} else if TRASH_SUPPORTED && (trash || (rm_always_trash && !permanent)) { } else if TRASH_SUPPORTED && (trash || (rm_always_trash && !permanent)) {
@ -384,10 +379,7 @@ fn rm(
))] ))]
{ {
trash::delete(&f).map_err(|e: trash::Error| { trash::delete(&f).map_err(|e: trash::Error| {
Error::new( Error::new(ErrorKind::Other, format!("{e:?}\nTry '--permanent' flag"))
ErrorKind::Other,
format!("{e:?}\nTry '--permanent' flag"),
)
}) })
} }
@ -425,48 +417,52 @@ fn rm(
if let Err(e) = result { if let Err(e) = result {
let msg = format!("Could not delete {:}: {e:}", f.to_string_lossy()); let msg = format!("Could not delete {:}: {e:}", f.to_string_lossy());
Value::error(ShellError::RemoveNotPossible { msg, span }, span) Err(ShellError::RemoveNotPossible { msg, span })
} else if verbose { } else if verbose {
let msg = if interactive && !confirmed { let msg = if interactive && !confirmed {
"not deleted" "not deleted"
} else { } else {
"deleted" "deleted"
}; };
let val = format!("{} {:}", msg, f.to_string_lossy()); Ok(Some(format!("{} {:}", msg, f.to_string_lossy())))
Value::string(val, span)
} else { } else {
Value::nothing(span) Ok(None)
} }
} else { } else {
let error = format!("Cannot remove {:}. try --recursive", f.to_string_lossy()); let error = format!("Cannot remove {:}. try --recursive", f.to_string_lossy());
Value::error( Err(ShellError::GenericError {
ShellError::GenericError {
error, error,
msg: "cannot remove non-empty directory".into(), msg: "cannot remove non-empty directory".into(),
span: Some(span), span: Some(span),
help: None, help: None,
inner: vec![], inner: vec![],
}, })
span,
)
} }
} else { } else {
let error = format!("no such file or directory: {:}", f.to_string_lossy()); let error = format!("no such file or directory: {:}", f.to_string_lossy());
Value::error( Err(ShellError::GenericError {
ShellError::GenericError {
error, error,
msg: "no such file or directory".into(), msg: "no such file or directory".into(),
span: Some(span), span: Some(span),
help: None, help: None,
inner: vec![], inner: vec![],
},
span,
)
}
}) })
.filter(|x| !matches!(x.get_type(), Type::Nothing)) }
.into_pipeline_data(span, ctrlc) });
.print_not_formatted(engine_state, false, true)?;
for result in iter {
if nu_utils::ctrl_c::was_pressed(&engine_state.ctrlc) {
return Err(ShellError::InterruptedByUser {
span: Some(call.head),
});
}
match result {
Ok(None) => {}
Ok(Some(msg)) => eprintln!("{msg}"),
Err(err) => report_error_new(engine_state, &err),
}
}
Ok(PipelineData::empty()) Ok(PipelineData::empty())
} }

View file

@ -877,32 +877,6 @@ impl PipelineData {
Ok(0) Ok(0)
} }
/// Consume and print self data immediately.
///
/// Unlike [`.print()`] does not call `table` to format data and just prints it
/// one element on a line
/// * `no_newline` controls if we need to attach newline character to output.
/// * `to_stderr` controls if data is output to stderr, when the value is false, the data is output to stdout.
pub fn print_not_formatted(
self,
engine_state: &EngineState,
no_newline: bool,
to_stderr: bool,
) -> Result<i64, ShellError> {
if let PipelineData::ExternalStream {
stdout: stream,
stderr: stderr_stream,
exit_code,
..
} = self
{
print_if_stream(stream, stderr_stream, to_stderr, exit_code)
} else {
let config = engine_state.get_config();
self.write_all_and_flush(engine_state, config, no_newline, to_stderr)
}
}
fn write_all_and_flush( fn write_all_and_flush(
self, self,
engine_state: &EngineState, engine_state: &EngineState,