cli: Fix and simplify `rad review`

* Properly remove review help message from review
* Remove `--no-confirm` and confirmation step

The CLI should be mostly non-interactive for day-to-day commands,
keeping the interactivity for the TUI.
This commit is contained in:
Alexis Sellier 2023-04-06 15:02:13 +02:00
parent f999a62da2
commit c1face7cd8
No known key found for this signature in database
2 changed files with 7 additions and 32 deletions

View File

@ -108,7 +108,7 @@ $ rad patch checkout 191a14e52
We can also add a review verdict as such: We can also add a review verdict as such:
``` ```
$ rad review 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 --accept --no-confirm --no-message --no-sync $ rad review 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 --accept --no-message --no-sync
✓ Patch 191a14e accepted ✓ Patch 191a14e accepted
``` ```

View File

@ -29,8 +29,6 @@ Options
-r, --revision <number> Revision number to review, defaults to the latest -r, --revision <number> Revision number to review, defaults to the latest
--[no-]sync Sync review to seed (default: sync) --[no-]sync Sync review to seed (default: sync)
-m, --message [<string>] Provide a comment with the review (default: prompt) -m, --message [<string>] Provide a comment with the review (default: prompt)
--no-confirm Don't ask for confirmation
--no-message Don't provide a comment with the review
--help Print help --help Print help
"#, "#,
}; };
@ -52,7 +50,6 @@ pub struct Options {
pub message: Message, pub message: Message,
pub sync: bool, pub sync: bool,
pub verbose: bool, pub verbose: bool,
pub confirm: bool,
pub verdict: Option<Verdict>, pub verdict: Option<Verdict>,
} }
@ -66,7 +63,6 @@ impl Args for Options {
let mut message = Message::default(); let mut message = Message::default();
let mut sync = true; let mut sync = true;
let mut verbose = false; let mut verbose = false;
let mut confirm = true;
let mut verdict = None; let mut verdict = None;
while let Some(arg) = parser.next()? { while let Some(arg) = parser.next()? {
@ -94,9 +90,6 @@ impl Args for Options {
message.append(&txt); message.append(&txt);
} }
} }
Long("no-confirm") => {
confirm = false;
}
Long("no-message") => { Long("no-message") => {
message = Message::Blank; message = Message::Blank;
} }
@ -121,7 +114,6 @@ impl Args for Options {
id: id.ok_or_else(|| anyhow!("a patch to review must be provided"))?, id: id.ok_or_else(|| anyhow!("a patch to review must be provided"))?,
message, message,
sync, sync,
confirm,
revision, revision,
verbose, verbose,
verdict, verdict,
@ -153,31 +145,14 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
.nth(revision_ix) .nth(revision_ix)
.ok_or_else(|| anyhow!("revision R{} does not exist", revision_ix))?; .ok_or_else(|| anyhow!("revision R{} does not exist", revision_ix))?;
let message = options.message.get(REVIEW_HELP_MSG); let message = options.message.get(REVIEW_HELP_MSG);
let message = message.replace(REVIEW_HELP_MSG.trim(), "");
let verdict_pretty = match options.verdict { let message = if message.is_empty() {
Some(Verdict::Accept) => term::format::highlight("Accept"), None
Some(Verdict::Reject) => term::format::negative("Reject"), } else {
None => term::format::dim("Review"), Some(message)
}; };
if options.confirm
&& !term::confirm(format!(
"{} {} {} by {}?",
verdict_pretty,
patch_id_pretty,
term::format::dim(format!("R{revision_ix}")),
term::format::tertiary(patch.author().id())
))
{
anyhow::bail!("Patch review aborted");
}
patch.review( patch.review(*revision_id, options.verdict, message, vec![], &signer)?;
*revision_id,
options.verdict,
Some(message),
vec![],
&signer,
)?;
match options.verdict { match options.verdict {
Some(Verdict::Accept) => { Some(Verdict::Accept) => {