Skip to content

Commit

Permalink
rust-remove: remove a regular file
Browse files Browse the repository at this point in the history
Summary:
## Background

We are migrating `eden remove` command from Python to Rust for better error handling and code readability.

## This Diff

Handles the case when the path provided by the user to remove is actually a file. To handle this, we prompt the user for confirmation and simply go deleting it.

Also in this diff, we introduced a testing-only flag "-n". This is because currently dialoguer::Confirm does not accept input from non-terminal.

Passing "-y" and "-n" at the same time is undefined and will cause error.

Reviewed By: muirdm

Differential Revision: D65801635

fbshipit-source-id: 8990a161936d0c132c97fe751562a90f319be30f
  • Loading branch information
lXXXw authored and facebook-github-bot committed Nov 20, 2024
1 parent f7caf40 commit cce45ac
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 8 deletions.
39 changes: 33 additions & 6 deletions eden/fs/cli_rs/edenfs-commands/src/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ pub struct RemoveCmd {
#[clap(short = 'q', long = "quiet", hide = true)]
suppress_output: bool,

// Answer no for any prompt.
// This is only used in testing the path when a user does not confirm upon the prompt
// I have to this because dialoguer::Confirm does not accept input from non-terminal
// https://github.com/console-rs/dialoguer/issues/170
//
// When provided with "-y": undefined!
#[clap(short = 'n', long = "answer-no", hide = true)]
no: bool,

#[clap(long, hide = true)]
preserve_mount_point: bool,
}
Expand Down Expand Up @@ -168,11 +177,16 @@ impl RegFile {
async fn next(&self, context: &mut RemoveContext) -> Result<Option<State>> {
if context
.io
.prompt_user("RegFile State is not implemented yet...".to_string())?
.prompt_user("{} is a file, do you still want to remove it?".to_string())?
{
return Err(anyhow!("Rust remove(RegFile) is not implemented!"));
fs::remove_file(context.canonical_path.as_path())
.with_context(|| format!("Failed to remove mount point {}", context))?;
return Ok(None);
}
Ok(None)

Err(anyhow!(
"User did not confirm the removal. Stopping. Nothing removed!"
))
}
}

Expand Down Expand Up @@ -404,7 +418,14 @@ impl Subcommand for RemoveCmd {
"Currently supporting only one path given per run"
);

let messenger = Messenger::new(IO::stdio(), self.skip_prompt, self.suppress_output);
if self.skip_prompt && self.no {
return Err(anyhow!(
"Both '-y' and '-n' are provided. This is not supported.\nExisting."
));
}

let messenger =
Messenger::new(IO::stdio(), self.skip_prompt, self.suppress_output, self.no);

let mut context =
RemoveContext::new(self.paths[0].clone(), self.preserve_mount_point, messenger);
Expand All @@ -431,13 +452,15 @@ impl Subcommand for RemoveCmd {
struct Messenger {
logger: TermLogger,
skip_prompt: bool,
answer_no: bool,
}

impl Messenger {
fn new(io: IO, skip_prompt: bool, suppress_output: bool) -> Messenger {
fn new(io: IO, skip_prompt: bool, suppress_output: bool, answer_no: bool) -> Messenger {
Messenger {
logger: TermLogger::new(&io).with_quiet(suppress_output),
skip_prompt,
answer_no,
}
}

Expand All @@ -462,6 +485,10 @@ impl Messenger {
}

fn prompt_user(&self, prompt: String) -> Result<bool> {
if self.answer_no {
return Ok(false);
}

if !self.skip_prompt {
self.logger.info(prompt);
let res = Confirm::new().with_prompt("Proceed?").interact()?;
Expand Down Expand Up @@ -513,7 +540,7 @@ mod tests {
}

fn default_context(original_path: String) -> RemoveContext {
let messenger = Messenger::new(IO::null(), true, true);
let messenger = Messenger::new(IO::null(), true, true, false);
RemoveContext {
original_path,
canonical_path: PathBuf::new(),
Expand Down
22 changes: 20 additions & 2 deletions eden/scm/tests/test-eden-remove.t
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,30 @@ setup backing repo
Cloning new repository at $TESTTMP/wcrepo...
Success. Checked out commit 00000000

Do not provide both '-y' and '-n'
$ EDENFSCTL_ONLY_RUST=true eden remove -q -y -n $TESTTMP/wcrepo/test_dir
Error: Both '-y' and '-n' are provided. This is not supported.
Existing.
[1]

touch a test file
$ touch $TESTTMP/wcrepo/file.txt

eden remove this file should see error about RegFile state
eden remove this file, answer no when there is prompt
$ EDENFSCTL_ONLY_RUST=true eden remove -q -n $TESTTMP/wcrepo/file.txt
Error: User did not confirm the removal. Stopping. Nothing removed!
[1]

the file is still there
$ ls $TESTTMP/wcrepo/file.txt | wc -l
1

eden remove this file, skip prompt with "yes"
$ EDENFSCTL_ONLY_RUST=true eden remove -q -y $TESTTMP/wcrepo/file.txt
Error: Rust remove(RegFile) is not implemented!

file is now gone
$ ls $TESTTMP/wcrepo/file.txt
ls: $TESTTMP/wcrepo/file.txt: $ENOENT$
[1]

create a test directory
Expand Down

0 comments on commit cce45ac

Please sign in to comment.