Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
cli: Improve patch command output
Alexis Sellier committed 3 years ago
commit 8dba01d6aeca3563279157835cb715941e959be6
parent 40eb6f2a6ee3ce531bf95f71cdf3882d72c101cb
9 files changed +131 -165
modified radicle-cli/examples/rad-patch.md
@@ -26,11 +26,8 @@ Once the code is ready, we open (or create) a patch with our changes for the pro

```
$ rad patch open --message "Define power requirements" --message "See details."
-
✓ Pushing HEAD to storage...
-
✓ Analyzing remotes...

master <- z6MknSL…StBU8Vi/flux-capacitor-power (3e674d1)
-

1 commit(s) ahead, 0 commit(s) behind

3e674d1 Define power requirements
@@ -46,11 +43,11 @@ It will now be listed as one of the project's open patches.

```
$ rad patch
-
╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
-
│ Define power requirements 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0 │
-
├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
-
│ ● opened by did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) 3 months ago                                │
-
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
+
╭─────────────────────────────────────────────────────────────────────────────────────────╮
+
│ Define power requirements 191a14e R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0   │
+
├─────────────────────────────────────────────────────────────────────────────────────────┤
+
│ ● opened by did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) 3 months ago │
+
╰─────────────────────────────────────────────────────────────────────────────────────────╯
```
```
$ rad patch show 191a14e520f2eeff7c0e3ee0a5523c5217eecb89
@@ -85,18 +82,10 @@ $ git commit --message "Add README, just for the fun"
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 README.md
$ rad patch update --message "Add README, just for the fun" 191a14e520f2eeff7c0e3ee0a5523c5217eecb89
-

-
🌱 Updating patch for heartwood
-

-
✓ Pushing HEAD to storage...
-
✓ Analyzing remotes...
-

-
191a14e520f R0 (3e674d1) -> R1 (27857ec)
+
191a14e R0 (3e674d1) -> R1 (27857ec)
1 commit(s) ahead, 0 commit(s) behind

-

-
✓ Patch 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 updated 🌱
-

+
✓ Patch 191a14e updated to 56a5efc062b34a97a162a3a4d4468ce3e7ba1c84
```

And let's leave a quick comment for our team:
@@ -112,24 +101,25 @@ Now, let's checkout the patch that we just created:

```
$ rad patch checkout 191a14e52
-
✓ Switched to branch patch/191a14e520f
+
✓ Switched to branch patch/191a14e
```

We can also add a review verdict as such:

```
$ rad review 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 --accept --no-confirm --no-message --no-sync
-
✓ Patch 191a14e520f accepted
+
✓ Patch 191a14e accepted
```

Showing the patch list now will reveal the favorable verdict:

```
$ rad patch
-
╭───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
-
│ Define power requirements 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 R1 27857ec (flux-capacitor-power, patch/191a14e520f) ahead 2, behind 0 │
-
├───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
-
│ ● opened by did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) 3 months ago                                                   │
-
│ ✓ accepted by z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) 3 months ago                                                         │
-
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
+
╭──────────────────────────────────────────────────────────────────────────────────────────────────────╮
+
│ Define power requirements 191a14e R1 27857ec (flux-capacitor-power, patch/191a14e) ahead 2, behind 0 │
+
├──────────────────────────────────────────────────────────────────────────────────────────────────────┤
+
│ ● opened by did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) 3 months ago              │
+
│ ↑ updated to 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 (3e674d1) 3 months ago                         │
+
│ ✓ accepted by z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) 3 months ago                    │
+
╰──────────────────────────────────────────────────────────────────────────────────────────────────────╯
```
modified radicle-cli/examples/workflow/4-patching-contributor.md
@@ -26,11 +26,8 @@ Once the code is ready, we open a patch with our changes.

```
$ rad patch open --message "Define power requirements" --message "See details."
-
✓ Pushing HEAD to storage...
-
✓ Analyzing remotes...

master <- z6Mkt67…v4N1tRk/flux-capacitor-power (3e674d1)
-

1 commit(s) ahead, 0 commit(s) behind

3e674d1 Define power requirements
@@ -46,11 +43,11 @@ It will now be listed as one of the project's open patches.

```
$ rad patch
-
╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
-
│ Define power requirements a07ef7743a32a2e902672ea3526d1db6ee08108a R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0 │
-
├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
-
│ ● opened by did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk (you) 3 months ago                                │
-
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
+
╭─────────────────────────────────────────────────────────────────────────────────────────╮
+
│ Define power requirements a07ef77 R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0   │
+
├─────────────────────────────────────────────────────────────────────────────────────────┤
+
│ ● opened by did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk (you) 3 months ago │
+
╰─────────────────────────────────────────────────────────────────────────────────────────╯
$ rad patch show a07ef7743a32a2e902672ea3526d1db6ee08108a
╭────────────────────────────────────────────────────────────────────╮
│ Title   Define power requirements                                  │
@@ -83,18 +80,10 @@ $ git commit --message "Add README, just for the fun"
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 README.md
$ rad patch update --message "Add README, just for the fun" a07ef7743a32a2e902672ea3526d1db6ee08108a
-

-
🌱 Updating patch for heartwood
-

-
✓ Pushing HEAD to storage...
-
✓ Analyzing remotes...
-

-
a07ef7743a3 R0 (3e674d1) -> R1 (27857ec)
+
a07ef77 R0 (3e674d1) -> R1 (27857ec)
1 commit(s) ahead, 0 commit(s) behind

-

-
✓ Patch a07ef7743a32a2e902672ea3526d1db6ee08108a updated 🌱
-

+
✓ Patch a07ef77 updated to 4f15eb5c994edd0bb6be29cb4801aa74308cc628
```

And let's leave a quick comment for our team:
modified radicle-cli/examples/workflow/5-patching-maintainer.md
@@ -27,11 +27,12 @@ $ git branch -r
  bob/master
  rad/master
$ rad patch
-
╭─────────────────────────────────────────────────────────────────────────────────────────────────╮
-
│ Define power requirements a07ef7743a32a2e902672ea3526d1db6ee08108a R1 27857ec ahead 2, behind 0 │
-
├─────────────────────────────────────────────────────────────────────────────────────────────────┤
-
│ ● opened by did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk 3 months ago               │
-
╰─────────────────────────────────────────────────────────────────────────────────────────────────╯
+
╭───────────────────────────────────────────────────────────────────────────────────╮
+
│ Define power requirements a07ef77 R1 27857ec ahead 2, behind 0                    │
+
├───────────────────────────────────────────────────────────────────────────────────┤
+
│ ● opened by did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk 3 months ago │
+
│ ↑ updated to a07ef7743a32a2e902672ea3526d1db6ee08108a (3e674d1) 3 months ago      │
+
╰───────────────────────────────────────────────────────────────────────────────────╯
```

Wait! There's a mistake.  The REQUIREMENTS should be a markdown file.  Let's
@@ -48,16 +49,8 @@ $ git commit --fixup HEAD~
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename REQUIREMENTS => REQUIREMENTS.md (100%)
$ rad patch update --message "Define power requirements" --message "See details." a07ef7743a32a2e902672ea3526d1db6ee08108a
-

-
🌱 Updating patch for heartwood
-

-
✓ Pushing HEAD to storage...
-
✓ Analyzing remotes...
-

-
a07ef7743a3 R1 (27857ec) -> R2 (f6484e0)
+
a07ef77 R1 (27857ec) -> R2 (f6484e0)
1 commit(s) ahead, 0 commit(s) behind

-

-
✓ Patch a07ef7743a32a2e902672ea3526d1db6ee08108a updated 🌱
-

+
✓ Patch a07ef77 updated to 737dc47d9eba730c5db8a8e33c41c7955f9093de
```
modified radicle-cli/src/commands/merge.rs
@@ -195,12 +195,11 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
        }
        MergeStyle::Commit
    } else if merge.is_up_to_date() {
-
        term::info!(
-
            "✓ Patch {} is already part of {}",
+
        term::success!(
+
            "Patch {} is already part of current branch {}",
            term::format::tertiary(patch_id),
-
            term::format::highlight(branch)
+
            term::format::parens(term::format::yellow(branch))
        );
-

        return Ok(());
    } else if merge.is_unborn() {
        anyhow::bail!("HEAD does not point to a valid commit");
@@ -223,14 +222,14 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
    };

    term::info!(
-
        "{} {} {} ({}) by {} into {} ({}) via {}...",
+
        "{} {} {} {} by {} into {} {} via {}...",
        term::format::bold("Merging"),
        term::format::tertiary(term::format::cob(&patch_id)),
        term::format::dim(format!("R{revision_ix}")),
-
        term::format::secondary(term::format::oid(revision.oid)),
+
        term::format::parens(term::format::secondary(term::format::oid(revision.oid))),
        term::format::tertiary(patch.author().id),
        term::format::highlight(branch),
-
        term::format::secondary(term::format::oid(head_oid)),
+
        term::format::parens(term::format::secondary(term::format::oid(head_oid))),
        merge_style_pretty
    );

modified radicle-cli/src/commands/patch/common.rs
@@ -43,7 +43,6 @@ pub fn get_merge_target(
    storage: &Repository,
    head_branch: &git::raw::Branch,
) -> anyhow::Result<(git::RefString, git::Oid)> {
-
    let spinner = term::spinner("Analyzing remotes...");
    let (qualified_ref, target_oid) = storage.canonical_head()?;
    let head_oid = branch_oid(head_branch)?;
    let merge_base = storage.raw().merge_base(*head_oid, *target_oid)?;
@@ -52,11 +51,7 @@ pub fn get_merge_target(
        anyhow::bail!("commits are already included in the target branch; nothing to do");
    }

-
    // TODO: Tell user how many peers don't have this change.
-
    spinner.finish();
-

-
    let branch = get_branch(qualified_ref);
-
    Ok((branch, (*target_oid).into()))
+
    Ok((get_branch(qualified_ref), (*target_oid).into()))
}

/// Return the [`Oid`] of the merge target.
@@ -146,13 +141,8 @@ pub fn push_to_storage(
    options: &Options,
) -> anyhow::Result<()> {
    let head_oid = branch_oid(head_branch)?;
-
    let mut spinner = term::spinner(format!(
-
        "Looking for HEAD ({}) in storage...",
-
        term::format::secondary(term::format::oid(head_oid))
-
    ));
    if storage.commit(head_oid).is_err() {
        if !options.push {
-
            spinner.failed();
            term::blank();

            return Err(Error::WithHint {
@@ -161,7 +151,6 @@ pub fn push_to_storage(
            }
            .into());
        }
-
        spinner.message("Pushing HEAD to storage...");

        let output = match head_branch.upstream() {
            Ok(_) => git::run::<_, _, &str, &str>(Path::new("."), ["push", "rad"], [])?,
@@ -172,14 +161,11 @@ pub fn push_to_storage(
            )?,
        };
        if options.verbose {
-
            spinner.finish();
            term::blob(output);

            return Ok(());
        }
    }
-
    spinner.finish();
-

    Ok(())
}

modified radicle-cli/src/commands/patch/create.rs
@@ -66,7 +66,6 @@ fn show_patch_commit_info(
        term::format::highlight(branch_name(head_branch)?),
        term::format::secondary(term::format::oid(head_oid)),
    );
-
    term::blank();

    // TODO: Test case where the target branch has been re-written passed the merge-base, since the fork was created
    // This can also happen *after* the patch is created.
modified radicle-cli/src/commands/patch/list.rs
@@ -75,14 +75,14 @@ fn print(
        &patch.timestamp(),
    )));

-
    let (_, revision) = patch
+
    let (latest, revision) = patch
        .latest()
        .ok_or_else(|| anyhow!("patch is malformed: no revisions found"))?;
    let mut widget = term::VStack::default()
        .child(
            term::Line::spaced([
                term::format::bold(patch.title()).into(),
-
                term::format::highlight(patch_id).into(),
+
                term::format::highlight(term::format::cob(patch_id)).into(),
                term::format::dim(format!("R{}", patch.version())).into(),
            ])
            .space()
@@ -99,62 +99,84 @@ fn print(
        .border(Some(term::colors::FAINT));

    let mut timeline = Vec::new();
-
    for merge in revision.merges.iter() {
-
        let peer = repository.remote(&merge.node)?;
-
        let mut badges = Vec::new();
-

-
        if peer.delegate {
-
            badges.push(term::format::secondary("(delegate)").into());
-
        }
-
        if peer.id == *whoami {
-
            badges.push(term::format::primary("(you)").into());
+
    for (revision_id, revision) in patch.revisions() {
+
        // Don't show an "update" line for the first revision.
+
        if revision_id != latest {
+
            timeline.push((
+
                revision.timestamp,
+
                term::Line::spaced(
+
                    [
+
                        term::format::tertiary("↑").into(),
+
                        term::format::default("updated to").into(),
+
                        term::format::dim(revision_id).into(),
+
                        term::format::parens(term::format::secondary(term::format::oid(
+
                            revision.oid,
+
                        )))
+
                        .into(),
+
                    ]
+
                    .into_iter(),
+
                ),
+
            ));
        }

-
        timeline.push((
-
            merge.timestamp,
-
            term::Line::spaced(
-
                [
-
                    term::format::secondary(term::format::dim("✓ merged")).into(),
-
                    term::format::default("by").into(),
-
                    term::format::tertiary(peer.id).into(),
-
                ]
-
                .into_iter()
-
                .chain(badges),
-
            ),
-
        ));
-
    }
-
    for (reviewer, review) in revision.reviews.iter() {
-
        let verdict = match review.verdict() {
-
            Some(Verdict::Accept) => term::format::positive(term::format::dim("✓ accepted")),
-
            Some(Verdict::Reject) => term::format::negative(term::format::dim("✗ rejected")),
-
            None => term::format::negative(term::format::dim("⋄ reviewed")),
-
        };
-
        let peer = repository.remote(reviewer)?;
-
        let mut badges = Vec::new();
-

-
        if peer.delegate {
-
            badges.push(term::format::secondary("(delegate)").into());
+
        for merge in revision.merges.iter() {
+
            let peer = repository.remote(&merge.node)?;
+
            let mut badges = Vec::new();
+

+
            if peer.delegate {
+
                badges.push(term::format::secondary("(delegate)").into());
+
            }
+
            if peer.id == *whoami {
+
                badges.push(term::format::primary("(you)").into());
+
            }
+

+
            timeline.push((
+
                merge.timestamp,
+
                term::Line::spaced(
+
                    [
+
                        term::format::primary("✓").bold().into(),
+
                        term::format::default("merged").into(),
+
                        term::format::default("by").into(),
+
                        term::format::tertiary(Did::from(peer.id)).into(),
+
                    ]
+
                    .into_iter()
+
                    .chain(badges),
+
                ),
+
            ));
        }
-
        if peer.id == *whoami {
-
            badges.push(term::format::primary("(you)").into());
+
        for (reviewer, review) in revision.reviews.iter() {
+
            let verdict = match review.verdict() {
+
                Some(Verdict::Accept) => term::format::positive(term::format::dim("✓ accepted")),
+
                Some(Verdict::Reject) => term::format::negative(term::format::dim("✗ rejected")),
+
                None => term::format::negative(term::format::dim("⋄ reviewed")),
+
            };
+
            let peer = repository.remote(reviewer)?;
+
            let mut badges = Vec::new();
+

+
            if peer.delegate {
+
                badges.push(term::format::secondary("(delegate)").into());
+
            }
+
            if peer.id == *whoami {
+
                badges.push(term::format::primary("(you)").into());
+
            }
+

+
            timeline.push((
+
                review.timestamp(),
+
                term::Line::spaced(
+
                    [
+
                        verdict.into(),
+
                        term::format::default("by").into(),
+
                        term::format::tertiary(reviewer).into(),
+
                    ]
+
                    .into_iter()
+
                    .chain(badges),
+
                ),
+
            ));
        }
-

-
        timeline.push((
-
            review.timestamp(),
-
            term::Line::spaced(
-
                [
-
                    verdict.into(),
-
                    term::format::default("by").into(),
-
                    term::format::tertiary(reviewer).into(),
-
                ]
-
                .into_iter()
-
                .chain(badges),
-
            ),
-
        ));
    }
    timeline.sort_by_key(|(t, _)| *t);

-
    for (time, mut line) in timeline.into_iter().rev() {
+
    for (time, mut line) in timeline.into_iter() {
        line.push(term::Label::space());
        line.push(term::format::dim(term::format::timestamp(&time)));
        widget.push(line);
modified radicle-cli/src/commands/patch/update.rs
@@ -1,5 +1,3 @@
-
use anyhow::Context;
-

use radicle::cob::patch;
use radicle::git;
use radicle::prelude::*;
@@ -11,7 +9,7 @@ use crate::terminal as term;

const REVISION_MSG: &str = r#"
<!--
-
Please enter a comment message for your patch update. Leaving this
+
Please enter a comment for your patch update. Leaving this
blank is also okay.
-->
"#;
@@ -26,27 +24,18 @@ fn select_patch(
    let head_oid = branch_oid(head_branch)?;
    let base_oid = workdir.merge_base(*target_oid, *head_oid)?;

-
    let mut spinner = term::spinner("Finding patches to update...");
    let mut result =
        find_unmerged_with_base(*head_oid, *target_oid, base_oid, patches, workdir, whoami)?;

-
    let Some((id, patch, _)) = result.pop() else {
-
        spinner.failed();
+
    let Some((id, _, _)) = result.pop() else {
        term::blank();
-
        anyhow::bail!("No patches found that share a base, please create a new patch or specify the patch id manually");
+
        anyhow::bail!("No patches found to update, please open a new patch or specify the patch id manually");
    };

    if !result.is_empty() {
-
        spinner.failed();
        term::blank();
        anyhow::bail!("More than one patch available to update, please specify an id with `rad patch --update <id>`");
    }
-
    spinner.message(format!(
-
        "Found existing patch {} {}",
-
        term::format::tertiary(term::format::cob(&id)),
-
        term::format::italic(patch.title())
-
    ));
-
    spinner.finish();
    term::blank();

    Ok(id)
@@ -63,20 +52,20 @@ fn show_update_commit_info(
    let current_version = patch.version();
    let head_oid = branch_oid(head_branch)?;

-
    term::blank();
    term::info!(
-
        "{} {} ({}) -> {} ({})",
+
        "{} {} {} -> {} {}",
        term::format::tertiary(term::format::cob(patch_id)),
        term::format::dim(format!("R{current_version}")),
-
        term::format::secondary(term::format::oid(current_revision.oid)),
+
        term::format::parens(term::format::secondary(term::format::oid(
+
            current_revision.oid
+
        ))),
        term::format::dim(format!("R{}", current_version + 1)),
-
        term::format::secondary(term::format::oid(*head_oid)),
+
        term::format::parens(term::format::secondary(term::format::oid(*head_oid))),
    );

    // Difference between the two revisions.
    let head_oid = branch_oid(head_branch)?;
    term::patch::print_commits_ahead_behind(workdir, *head_oid, *current_revision.oid)?;
-
    term::blank();

    Ok(())
}
@@ -90,18 +79,9 @@ pub fn run(
    message: term::patch::Message,
    options: &Options,
) -> anyhow::Result<()> {
-
    let project = storage.project_of(&profile.public_key).context(format!(
-
        "couldn't load project {} from local state",
-
        storage.id
-
    ))?;
    // `HEAD`; This is what we are proposing as a patch.
    let head_branch = try_branch(workdir.head()?)?;

-
    term::headline(format!(
-
        "🌱 Updating patch for {}",
-
        term::format::highlight(project.name())
-
    ));
-

    push_to_storage(storage, &head_branch, options)?;

    let (_, target_oid) = get_merge_target(storage, &head_branch)?;
@@ -128,11 +108,14 @@ pub fn run(
    let base_oid = workdir.merge_base(*target_oid, *head_oid)?;
    let message = message.get(REVISION_MSG);
    let signer = term::signer(profile)?;
-
    patch.update(message, base_oid, *head_oid, &signer)?;
+
    let revision = patch.update(message, base_oid, *head_oid, &signer)?;

    term::blank();
-
    term::success!("Patch {} updated 🌱", term::format::highlight(patch_id));
-
    term::blank();
+
    term::success!(
+
        "Patch {} updated to {}",
+
        term::format::highlight(term::format::cob(&patch_id)),
+
        term::format::tertiary(revision),
+
    );

    if options.announce {
        // TODO
modified radicle-cli/src/terminal/format.rs
@@ -23,9 +23,14 @@ pub fn oid(oid: impl Into<radicle::git::Oid>) -> String {
    format!("{:.7}", oid.into())
}

+
/// Wrap parenthesis around styled input, eg. `"input"` -> `"(input)"`.
+
pub fn parens<D: fmt::Display>(input: Paint<D>) -> Paint<String> {
+
    Paint::new(format!("({})", input.item)).with_style(input.style)
+
}
+

/// Format a COB id.
pub fn cob(id: &ObjectId) -> String {
-
    format!("{:.11}", id.to_string())
+
    format!("{:.7}", id.to_string())
}

/// Format a timestamp.