Ctrl+c interruption - cp command (#8219)

# Description
if you try to copy a big file with `cp` you will noticed that you can't
interrupt the process. This pull request fix that.
This was discuss here
https://github.com/nushell/nushell/pull/8012#issuecomment-1427313054

# User-Facing Changes
None

# Tests + Formatting
- Check - `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- Check - `cargo clippy --workspace -- -D warnings -D
clippy::unwrap_used -A clippy::needless_collect` to check that you're
using the standard code style
- Check -  `cargo test --workspace` to check that all tests pass

---------

Co-authored-by: Reilly Wood <reilly.wood@icloud.com>
This commit is contained in:
Xoffio 2023-02-26 15:18:20 -05:00 committed by GitHub
parent 2492165fcb
commit 4b3e3a37a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -1,6 +1,8 @@
use std::fs::read_link; use std::fs::read_link;
use std::io::{BufReader, BufWriter, ErrorKind, Read, Write}; use std::io::{BufReader, BufWriter, ErrorKind, Read, Write};
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::atomic::AtomicBool;
use std::sync::Arc;
use nu_engine::env::current_dir; use nu_engine::env::current_dir;
use nu_engine::CallExt; use nu_engine::CallExt;
@ -151,6 +153,10 @@ impl Command for Cp {
let mut result = Vec::new(); let mut result = Vec::new();
for entry in sources.into_iter().flatten() { for entry in sources.into_iter().flatten() {
if nu_utils::ctrl_c::was_pressed(&ctrlc) {
return Ok(PipelineData::empty());
}
let mut sources = FileStructure::new(); let mut sources = FileStructure::new();
sources.walk_decorate(&entry, engine_state, stack)?; sources.walk_decorate(&entry, engine_state, stack)?;
@ -190,18 +196,19 @@ impl Command for Cp {
src, src,
dst, dst,
span, span,
&ctrlc,
copy_file_with_progressbar, copy_file_with_progressbar,
) )
} else { } else {
interactive_copy(interactive, src, dst, span, copy_file) interactive_copy(interactive, src, dst, span, &None, copy_file)
} }
} else if progress { } else if progress {
// uses std::io::copy to get the progress // use std::io::copy to get the progress
// slower std::fs::copy but useful if user needs to see the progress // slower then std::fs::copy but useful if user needs to see the progress
copy_file_with_progressbar(src, dst, span) copy_file_with_progressbar(src, dst, span, &ctrlc)
} else { } else {
// uses std::fs::copy // use std::fs::copy
copy_file(src, dst, span) copy_file(src, dst, span, &None)
}; };
result.push(res); result.push(res);
} }
@ -268,6 +275,11 @@ impl Command for Cp {
})?; })?;
for (s, d) in sources { for (s, d) in sources {
// Check if the user has pressed ctrl+c before copying a file
if nu_utils::ctrl_c::was_pressed(&ctrlc) {
return Ok(PipelineData::empty());
}
if s.is_dir() && !d.exists() { if s.is_dir() && !d.exists() {
std::fs::create_dir_all(&d).map_err(|e| { std::fs::create_dir_all(&d).map_err(|e| {
ShellError::GenericError( ShellError::GenericError(
@ -281,9 +293,9 @@ impl Command for Cp {
} }
if s.is_symlink() && not_follow_symlink { if s.is_symlink() && not_follow_symlink {
let res = if interactive && d.exists() { let res = if interactive && d.exists() {
interactive_copy(interactive, s, d, span, copy_symlink) interactive_copy(interactive, s, d, span, &None, copy_symlink)
} else { } else {
copy_symlink(s, d, span) copy_symlink(s, d, span, &None)
}; };
result.push(res); result.push(res);
} else if s.is_file() { } else if s.is_file() {
@ -294,15 +306,16 @@ impl Command for Cp {
s, s,
d, d,
span, span,
&ctrlc,
copy_file_with_progressbar, copy_file_with_progressbar,
) )
} else { } else {
interactive_copy(interactive, s, d, span, copy_file) interactive_copy(interactive, s, d, span, &None, copy_file)
} }
} else if progress { } else if progress {
copy_file_with_progressbar(s, d, span) copy_file_with_progressbar(s, d, span, &ctrlc)
} else { } else {
copy_file(s, d, span) copy_file(s, d, span, &None)
}; };
result.push(res); result.push(res);
}; };
@ -357,7 +370,8 @@ fn interactive_copy(
src: PathBuf, src: PathBuf,
dst: PathBuf, dst: PathBuf,
span: Span, span: Span,
copy_impl: impl Fn(PathBuf, PathBuf, Span) -> Value, _ctrl_status: &Option<Arc<AtomicBool>>,
copy_impl: impl Fn(PathBuf, PathBuf, Span, &Option<Arc<AtomicBool>>) -> Value,
) -> Value { ) -> Value {
let (interaction, confirmed) = try_interaction( let (interaction, confirmed) = try_interaction(
interactive, interactive,
@ -377,14 +391,20 @@ fn interactive_copy(
let msg = format!("{:} not copied to {:}", src.display(), dst.display()); let msg = format!("{:} not copied to {:}", src.display(), dst.display());
Value::String { val: msg, span } Value::String { val: msg, span }
} else { } else {
copy_impl(src, dst, span) copy_impl(src, dst, span, &None)
} }
} }
// This uses `std::fs::copy` to copy a file. There is another function called `copy_file_with_progressbar` // This uses `std::fs::copy` to copy a file. There is another function called `copy_file_with_progressbar`
// which uses `read` and `write` instead. This is to get the progress of the copy. Try to keep the logic in // which uses `read` and `write` instead. This is to get the progress of the copy. Try to keep the logic in
// this function in sync with `copy_file_with_progressbar` // this function in sync with `copy_file_with_progressbar`
fn copy_file(src: PathBuf, dst: PathBuf, span: Span) -> Value { // FIXME: `std::fs::copy` can't be interrupted. Consider using something else
fn copy_file(
src: PathBuf,
dst: PathBuf,
span: Span,
_ctrlc_status: &Option<Arc<AtomicBool>>,
) -> Value {
match std::fs::copy(&src, &dst) { match std::fs::copy(&src, &dst) {
Ok(_) => { Ok(_) => {
let msg = format!("copied {:} to {:}", src.display(), dst.display()); let msg = format!("copied {:} to {:}", src.display(), dst.display());
@ -397,7 +417,12 @@ fn copy_file(src: PathBuf, dst: PathBuf, span: Span) -> Value {
// This uses `read` and `write` to copy a file. There is another function called `copy_file` // This uses `read` and `write` to copy a file. There is another function called `copy_file`
// which uses `std::fs::copy` instead which is faster but does not provide progress updates for the copy. try to keep the // which uses `std::fs::copy` instead which is faster but does not provide progress updates for the copy. try to keep the
// logic in this function in sync with `copy_file` // logic in this function in sync with `copy_file`
fn copy_file_with_progressbar(src: PathBuf, dst: PathBuf, span: Span) -> Value { fn copy_file_with_progressbar(
src: PathBuf,
dst: PathBuf,
span: Span,
ctrlc_status: &Option<Arc<AtomicBool>>,
) -> Value {
let mut bytes_processed: u64 = 0; let mut bytes_processed: u64 = 0;
let mut process_failed: Option<std::io::Error> = None; let mut process_failed: Option<std::io::Error> = None;
@ -422,6 +447,12 @@ fn copy_file_with_progressbar(src: PathBuf, dst: PathBuf, span: Span) -> Value {
let mut buf_writer = BufWriter::new(file_out); let mut buf_writer = BufWriter::new(file_out);
loop { loop {
if nu_utils::ctrl_c::was_pressed(ctrlc_status) {
let err = std::io::Error::new(ErrorKind::Interrupted, "Interrupted");
process_failed = Some(err);
break;
}
// Read src file // Read src file
match buf_reader.read(&mut buffer) { match buf_reader.read(&mut buffer) {
// src file read successfully // src file read successfully
@ -457,7 +488,11 @@ fn copy_file_with_progressbar(src: PathBuf, dst: PathBuf, span: Span) -> Value {
// If copying the file failed // If copying the file failed
if let Some(error) = process_failed { if let Some(error) = process_failed {
if error.kind() == ErrorKind::Interrupted {
bar.abandoned_msg("# !! Interrupted !!".to_owned());
} else {
bar.abandoned_msg("# !! Error !!".to_owned()); bar.abandoned_msg("# !! Error !!".to_owned());
}
return convert_io_error(error, src, dst, span); return convert_io_error(error, src, dst, span);
} }
@ -472,7 +507,12 @@ fn copy_file_with_progressbar(src: PathBuf, dst: PathBuf, span: Span) -> Value {
Value::String { val: msg, span } Value::String { val: msg, span }
} }
fn copy_symlink(src: PathBuf, dst: PathBuf, span: Span) -> Value { fn copy_symlink(
src: PathBuf,
dst: PathBuf,
span: Span,
_ctrlc_status: &Option<Arc<AtomicBool>>,
) -> Value {
let target_path = read_link(src.as_path()); let target_path = read_link(src.as_path());
let target_path = match target_path { let target_path = match target_path {
Ok(p) => p, Ok(p) => p,