Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm: Added descend messages for interactive mode Fixes #3817 #3931

Merged
merged 10 commits into from
Oct 5, 2022
31 changes: 29 additions & 2 deletions src/uu/rm/src/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,36 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
}
} else {
let mut dirs: VecDeque<DirEntry> = VecDeque::new();
// The Paths to not descend into. We need to this because WalkDir doesn't have a way, afaik, to not descend into a directory
// So we have to just ignore paths as they come up if they start with a path we aren't descending into
let mut not_descended: Vec<PathBuf> = Vec::new();

for entry in WalkDir::new(path) {
'outer: for entry in WalkDir::new(path) {
match entry {
Ok(entry) => {
if options.interactive == InteractiveMode::Always {
for not_descend in &not_descended {
if entry.path().starts_with(not_descend) {
// We don't need to continue the rest of code in this loop if we are in a directory we don't want to descend into
continue 'outer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment explaining why we do that here

}
}
}
let file_type = entry.file_type();
if file_type.is_dir() {
dirs.push_back(entry);
// If we are in Interactive Mode Always and the directory isn't empty we ask if we should descend else we push this directory onto dirs vector
if options.interactive == InteractiveMode::Always
&& fs::read_dir(entry.path()).unwrap().count() != 0
{
// If we don't descend we push this directory onto our not_descended vector else we push this directory onto dirs vector
if prompt_descend(entry.path()) {
dirs.push_back(entry);
} else {
not_descended.push(entry.path().to_path_buf());
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment here. it is starting to be quite complex with the various if and else

dirs.push_back(entry);
}
} else {
had_err = remove_file(entry.path(), options).bitor(had_err);
}
Expand Down Expand Up @@ -447,6 +470,10 @@ fn prompt_write_protected(path: &Path, is_dir: bool, options: &Options) -> bool
}
}

fn prompt_descend(path: &Path) -> bool {
prompt(&(format!("rm: descend into directory {}? ", path.quote())))
palaster marked this conversation as resolved.
Show resolved Hide resolved
}

fn prompt_file(path: &Path, is_dir: bool) -> bool {
if is_dir {
prompt(&(format!("rm: remove directory {}? ", path.quote())))
Expand Down
47 changes: 47 additions & 0 deletions tests/by-util/test_rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,53 @@ fn test_rm_interactive_never() {
assert!(!at.file_exists(file_2));
}

#[test]
fn test_rm_descend_directory() {
// This test descends into each directory and deletes the files and folders inside of them
// This test will have the rm process asks 6 question and us answering Y to them will delete all the files and folders
use std::io::Write;
use std::process::Child;

// Needed for talking with stdin on platforms where CRLF or LF matters
const END_OF_LINE: &str = if cfg!(windows) { "\r\n" } else { "\n" };

let yes = format!("y{}", END_OF_LINE);

let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;

let file_1 = "a/at.txt";
let file_2 = "a/b/bt.txt";

at.mkdir_all("a/b/");
at.touch(file_1);
at.touch(file_2);

let mut child: Child = scene.ucmd().arg("-ri").arg("a").run_no_wait();

// Needed so that we can talk to the rm program
let mut child_stdin = child.stdin.take().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please document why it is needed

child_stdin.write_all(yes.as_bytes()).unwrap();
child_stdin.flush().unwrap();
child_stdin.write_all(yes.as_bytes()).unwrap();
child_stdin.flush().unwrap();
child_stdin.write_all(yes.as_bytes()).unwrap();
child_stdin.flush().unwrap();
child_stdin.write_all(yes.as_bytes()).unwrap();
child_stdin.flush().unwrap();
child_stdin.write_all(yes.as_bytes()).unwrap();
child_stdin.flush().unwrap();
child_stdin.write_all(yes.as_bytes()).unwrap();
child_stdin.flush().unwrap();

child.wait_with_output().unwrap();

assert!(!at.dir_exists("a/b"));
assert!(!at.dir_exists("a"));
assert!(!at.file_exists(file_1));
assert!(!at.file_exists(file_2));
}

#[test]
#[ignore = "issue #3722"]
fn test_rm_directory_rights_rm1() {
Expand Down