diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index 505ccaa1d7..8b3b7ddbd0 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -64,221 +64,59 @@ impl Command for Save { let span = call.head; let path = call.req::>(engine_state, stack, 0)?; - let arg_span = path.span; - let path = Path::new(&path.item); - - let path_exists = path.exists(); - if path_exists && !force && !append { - return Err(ShellError::GenericError( - "Destination file already exists".into(), - format!( - "Destination file '{}' already exists", - path.to_string_lossy() - ), - Some(arg_span), - Some("you can use -f, --force to force overwriting the destination".into()), - Vec::new(), - )); - } - - let file = match (append, path_exists) { - (true, true) => std::fs::OpenOptions::new() - .write(true) - .append(true) - .open(path), - _ => std::fs::File::create(path), - }; - - let mut file = match file { - Ok(file) => file, - Err(err) => { - return Err(ShellError::GenericError( - "Permission denied".into(), - err.to_string(), - Some(arg_span), - None, - Vec::new(), - )); - } - }; let stderr_path = call.get_flag::>(engine_state, stack, "stderr")?; - let stderr_file = match stderr_path { - None => None, - Some(stderr_path) => { - let stderr_span = stderr_path.span; - let stderr_path = Path::new(&stderr_path.item); - if stderr_path == path { - Some(file.try_clone()?) + + match input { + PipelineData::ExternalStream { stdout: None, .. } => { + // Open files to possibly truncate them + let _ = get_files(&path, &stderr_path, append, force)?; + Ok(PipelineData::empty()) + } + PipelineData::ExternalStream { + stdout: Some(stream), + stderr, + .. + } => { + let (file, stderr_file) = get_files(&path, &stderr_path, append, force)?; + + // delegate a thread to redirect stderr to result. + let handler = stderr.map(|stderr_stream| match stderr_file { + Some(stderr_file) => { + std::thread::spawn(move || stream_to_file(stderr_stream, stderr_file, span)) + } + None => std::thread::spawn(move || { + let _ = stderr_stream.into_bytes(); + Ok(PipelineData::empty()) + }), + }); + + let res = stream_to_file(stream, file, span); + if let Some(h) = handler { + h.join().map_err(|err| { + ShellError::ExternalCommand( + "Fail to receive external commands stderr message".to_string(), + format!("{err:?}"), + span, + ) + })??; + res } else { - match std::fs::File::create(stderr_path) { - Ok(file) => Some(file), - Err(err) => { - return Err(ShellError::GenericError( - "Permission denied".into(), - err.to_string(), - Some(stderr_span), - None, - Vec::new(), - )) - } - } + res } } - }; + input => { + let bytes = + input_to_bytes(input, Path::new(&path.item), raw, engine_state, stack, span)?; - let ext = if raw { - None - // if is extern stream , in other words , not value - } else if let PipelineData::ExternalStream { .. } = input { - None - } else if let PipelineData::Value(Value::String { .. }, ..) = input { - None - } else { - path.extension() - .map(|name| name.to_string_lossy().to_string()) - }; + // Only open file after successful conversion + let (mut file, _) = get_files(&path, &stderr_path, append, force)?; - if let Some(ext) = ext { - let output = match engine_state.find_decl(format!("to {}", ext).as_bytes(), &[]) { - Some(converter_id) => { - let output = engine_state.get_decl(converter_id).run( - engine_state, - stack, - &Call::new(span), - input, - )?; + file.write_all(&bytes) + .map_err(|err| ShellError::IOError(err.to_string()))?; - output.into_value(span) - } - None => input.into_value(span), - }; + file.flush()?; - match output { - Value::String { val, .. } => { - if let Err(err) = file.write_all(val.as_bytes()) { - return Err(ShellError::IOError(err.to_string())); - } else { - file.flush()? - } - - Ok(PipelineData::empty()) - } - Value::Binary { val, .. } => { - if let Err(err) = file.write_all(&val) { - return Err(ShellError::IOError(err.to_string())); - } else { - file.flush()? - } - - Ok(PipelineData::empty()) - } - Value::List { vals, .. } => { - let val = vals - .into_iter() - .map(|it| it.as_string()) - .collect::, ShellError>>()? - .join("\n") - + "\n"; - - if let Err(err) = file.write_all(val.as_bytes()) { - return Err(ShellError::IOError(err.to_string())); - } else { - file.flush()? - } - - Ok(PipelineData::empty()) - } - // Propagate errors by explicitly matching them before the final case. - Value::Error { error } => Err(error), - other => Err(ShellError::OnlySupportsThisInputType( - "string, binary or list".into(), - other.get_type().to_string(), - span, - // This line requires the Value::Error match above. - other.expect_span(), - )), - } - } else { - match input { - PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::empty()), - PipelineData::ExternalStream { - stdout: Some(stream), - stderr, - .. - } => { - // delegate a thread to redirect stderr to result. - let handler = stderr.map(|stderr_stream| match stderr_file { - Some(stderr_file) => std::thread::spawn(move || { - stream_to_file(stderr_stream, stderr_file, span) - }), - None => std::thread::spawn(move || { - let _ = stderr_stream.into_bytes(); - Ok(PipelineData::empty()) - }), - }); - - let res = stream_to_file(stream, file, span); - if let Some(h) = handler { - match h.join() { - Err(err) => { - return Err(ShellError::ExternalCommand( - "Fail to receive external commands stderr message".to_string(), - format!("{err:?}"), - span, - )) - } - Ok(res) => res, - }?; - res - } else { - res - } - } - input => match input.into_value(span) { - Value::String { val, .. } => { - if let Err(err) = file.write_all(val.as_bytes()) { - return Err(ShellError::IOError(err.to_string())); - } else { - file.flush()? - } - - Ok(PipelineData::empty()) - } - Value::Binary { val, .. } => { - if let Err(err) = file.write_all(&val) { - return Err(ShellError::IOError(err.to_string())); - } else { - file.flush()? - } - - Ok(PipelineData::empty()) - } - Value::List { vals, .. } => { - let val = vals - .into_iter() - .map(|it| it.as_string()) - .collect::, ShellError>>()? - .join("\n") - + "\n"; - - if let Err(err) = file.write_all(val.as_bytes()) { - return Err(ShellError::IOError(err.to_string())); - } else { - file.flush()? - } - - Ok(PipelineData::empty()) - } - // Propagate errors by explicitly matching them before the final case. - Value::Error { error } => Err(error), - other => Err(ShellError::OnlySupportsThisInputType( - "string, binary or list".into(), - other.get_type().to_string(), - span, - // This line requires the Value::Error match above. - other.expect_span(), - )), - }, + Ok(PipelineData::empty()) } } } @@ -314,6 +152,182 @@ impl Command for Save { } } +/// Convert [`PipelineData`] bytes to write in file, possibly converting +/// to format of output file +fn input_to_bytes( + input: PipelineData, + path: &Path, + raw: bool, + engine_state: &EngineState, + stack: &mut Stack, + span: Span, +) -> Result, ShellError> { + let ext = if raw { + None + // if is extern stream , in other words , not value + } else if let PipelineData::ExternalStream { .. } = input { + None + } else if let PipelineData::Value(Value::String { .. }, ..) = input { + None + } else { + path.extension() + .map(|name| name.to_string_lossy().to_string()) + }; + + if let Some(ext) = ext { + convert_to_extension(engine_state, &ext, stack, input, span) + } else { + let value = input.into_value(span); + string_binary_list_value_to_bytes(value, span) + } +} + +/// Convert given data into content of file of specified extension if +/// corresponding `to` command exists. Otherwise attempt to convert +/// data to bytes as is +fn convert_to_extension( + engine_state: &EngineState, + extension: &str, + stack: &mut Stack, + input: PipelineData, + span: Span, +) -> Result, ShellError> { + let converter = engine_state.find_decl(format!("to {}", extension).as_bytes(), &[]); + + let output = match converter { + Some(converter_id) => { + let output = engine_state.get_decl(converter_id).run( + engine_state, + stack, + &Call::new(span), + input, + )?; + + output.into_value(span) + } + None => input.into_value(span), + }; + + string_binary_list_value_to_bytes(output, span) +} + +/// Convert [`Value::String`] [`Value::Binary`] or [`Value::List`] into [`Vec`] of bytes +/// +/// Propagates [`Value::Error`] and creates error otherwise +fn string_binary_list_value_to_bytes(value: Value, span: Span) -> Result, ShellError> { + match value { + Value::String { val, .. } => Ok(val.into_bytes()), + Value::Binary { val, .. } => Ok(val), + Value::List { vals, .. } => { + let val = vals + .into_iter() + .map(|it| it.as_string()) + .collect::, ShellError>>()? + .join("\n") + + "\n"; + + Ok(val.into_bytes()) + } + // Propagate errors by explicitly matching them before the final case. + Value::Error { error } => Err(error), + other => Err(ShellError::OnlySupportsThisInputType( + "string, binary or list".into(), + other.get_type().to_string(), + span, + // This line requires the Value::Error match above. + other.expect_span(), + )), + } +} + +/// Convert string path to [`Path`] and [`Span`] and check if this path +/// can be used with given flags +fn prepare_path( + path: &Spanned, + append: bool, + force: bool, +) -> Result<(&Path, Span), ShellError> { + let span = path.span; + let path = Path::new(&path.item); + + if !(force || append) && path.exists() { + Err(ShellError::GenericError( + "Destination file already exists".into(), + format!( + "Destination file '{}' already exists", + path.to_string_lossy() + ), + Some(span), + Some("you can use -f, --force to force overwriting the destination".into()), + Vec::new(), + )) + } else { + Ok((path, span)) + } +} + +fn open_file(path: &Path, span: Span, append: bool) -> Result { + let file = match (append, path.exists()) { + (true, true) => std::fs::OpenOptions::new() + .write(true) + .append(true) + .open(path), + _ => std::fs::File::create(path), + }; + + file.map_err(|err| { + ShellError::GenericError( + "Permission denied".into(), + err.to_string(), + Some(span), + None, + Vec::new(), + ) + }) +} + +fn clone_file(file: &File, span: Span) -> Result { + file.try_clone().map_err(|err| { + ShellError::GenericError( + "Permission denied".into(), + err.to_string(), + Some(span), + None, + Vec::new(), + ) + }) +} + +/// Get output file and optional stderr file +fn get_files( + path: &Spanned, + stderr_path: &Option>, + append: bool, + force: bool, +) -> Result<(File, Option), ShellError> { + // First check both paths + let (path, path_span) = prepare_path(path, append, force)?; + let stderr_path_and_span = stderr_path + .as_ref() + .map(|stderr_path| prepare_path(stderr_path, append, force)) + .transpose()?; + + // Only if both files can be used open and possibly truncate them + let file = open_file(path, path_span, append)?; + + let stderr_file = stderr_path_and_span + .map(|(stderr_path, stderr_path_span)| { + if path == stderr_path { + clone_file(&file, stderr_path_span) + } else { + open_file(stderr_path, stderr_path_span, append) + } + }) + .transpose()?; + + Ok((file, stderr_file)) +} + fn stream_to_file( mut stream: RawStream, file: File, diff --git a/crates/nu-command/tests/commands/save.rs b/crates/nu-command/tests/commands/save.rs index 604d448b8c..40e3acabf1 100644 --- a/crates/nu-command/tests/commands/save.rs +++ b/crates/nu-command/tests/commands/save.rs @@ -177,3 +177,90 @@ fn save_override_works() { assert_eq!(actual, "abcd"); }) } + +#[test] +fn save_failure_not_overrides() { + Playground::setup("save_test_10", |dirs, sandbox| { + sandbox.with_files(vec![Stub::FileWithContent("result.toml", "Old content")]); + + let expected_file = dirs.test().join("result.toml"); + nu!( + cwd: dirs.root(), + // Writing number to file as toml fails + r#"3 | save save_test_10/result.toml -f"# + ); + let actual = file_contents(expected_file); + assert_eq!(actual, "Old content"); + }) +} + +#[test] +fn save_append_works_on_stderr() { + Playground::setup("save_test_11", |dirs, sandbox| { + sandbox.with_files(vec![ + Stub::FileWithContent("log.txt", "Old"), + Stub::FileWithContent("err.txt", "Old Err"), + ]); + + let expected_file = dirs.test().join("log.txt"); + let expected_stderr_file = dirs.test().join("err.txt"); + + nu!( + cwd: dirs.root(), + r#" + let-env FOO = " New"; + let-env BAZ = " New Err"; + do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -a -r save_test_11/log.txt --stderr save_test_11/err.txt"#, + ); + + let actual = file_contents(expected_file); + assert_eq!(actual, "Old New\n"); + + let actual = file_contents(expected_stderr_file); + assert_eq!(actual, "Old Err New Err\n"); + }) +} + +#[test] +fn save_not_overrides_err_by_default() { + Playground::setup("save_test_12", |dirs, sandbox| { + sandbox.with_files(vec![Stub::FileWithContent("err.txt", "Old Err")]); + + let actual = nu!( + cwd: dirs.root(), + r#" + let-env FOO = " New"; + let-env BAZ = " New Err"; + do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_12/log.txt --stderr save_test_12/err.txt"#, + ); + + assert!(actual.err.contains("Destination file already exists")); + }) +} + +#[test] +fn save_override_works_stderr() { + Playground::setup("save_test_13", |dirs, sandbox| { + sandbox.with_files(vec![ + Stub::FileWithContent("log.txt", "Old"), + Stub::FileWithContent("err.txt", "Old Err"), + ]); + + let expected_file = dirs.test().join("log.txt"); + let expected_stderr_file = dirs.test().join("err.txt"); + + nu!( + cwd: dirs.root(), + r#" + let-env FOO = "New"; + let-env BAZ = "New Err"; + do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -f -r save_test_13/log.txt --stderr save_test_13/err.txt"#, + ); + + let actual = file_contents(expected_file); + assert_eq!(actual, "New\n"); + + let actual = file_contents(expected_stderr_file); + assert_eq!(actual, "New Err\n"); + }) +}