cli: inline patch revision updates

Instead of having a timeline of a patch be global and sorted by
timestamp, organise it via revisions. Each revision has an entry in
the timeline, sorted by timestamp. Under each revision, a series of
updates are inlined, also sorted by their timestamps. Currently, these
updates can either be reviews or merges.

For example:

```
├────────────────────────────────────────────────────────────────────────────┤
│ ● opened by fintohaps (you) 1 week ago                                     │
│   └── ✓ accepted by fintohaps (you) 1 week ago                             │
│ ↑ updated to 5f2a123e3f2aa60f2e03815df283563495b73085 (a1c3c0f) 1 week ago │
│   └── ✓ accepted by fintohaps (you) 1 week ago                             │
│   └── ✓ merged by cloudhead (z6Mkt67…v4N1tRk) now                          │
╰────────────────────────────────────────────────────────────────────────────╯
```

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
This commit is contained in:
Fintan Halpenny 2024-01-03 14:39:22 +00:00 committed by cloudhead
parent 3323082ce3
commit a8cca57c0a
No known key found for this signature in database
6 changed files with 329 additions and 99 deletions

View File

@ -75,6 +75,21 @@ $ rad patch --merged
│ ✔ [ ... ] Second change alice (you) daf349f +0 -0 now │
│ ✔ [ ... ] First change alice (you) 20aa5dd +0 -0 now │
╰────────────────────────────────────────────────────────────────────╯
$ rad patch show 8357a9f1d61e80309d314491aa754969d9f47d77
╭────────────────────────────────────────────────────────────────╮
│ Title Second change │
│ Patch 8357a9f1d61e80309d314491aa754969d9f47d77 │
│ Author alice (you) │
│ Head daf349ff76bedf48c5f292290b682ee7be0683cf │
│ Branches feature/2 │
│ Commits ahead 0, behind 2 │
│ Status merged │
├────────────────────────────────────────────────────────────────┤
│ daf349f Second change │
├────────────────────────────────────────────────────────────────┤
│ ● opened by alice (you) now │
│ └─ ✓ merged by alice (you) at revision 8357a9f (d6399c7) now │
╰────────────────────────────────────────────────────────────────╯
```
We can verify that the remote tracking branches were also deleted:

View File

@ -179,7 +179,7 @@ $ rad patch show 6ff4f09
├─────────────────────────────────────────────────────────────────────┤
│ ● opened by z6MknSL…StBU8Vi (you) now │
│ ↑ updated to e0fd9f00b51e10e1ca88868e68e46e859ed371d7 (27857ec) now │
│ ✓ accepted by z6MknSL…StBU8Vi (you) now
└─ ✓ accepted by z6MknSL…StBU8Vi (you) now │
╰─────────────────────────────────────────────────────────────────────╯
```
@ -205,6 +205,6 @@ $ rad patch show 6ff4f09
├─────────────────────────────────────────────────────────────────────┤
│ ● opened by z6MknSL…StBU8Vi (you) now │
│ ↑ updated to e0fd9f00b51e10e1ca88868e68e46e859ed371d7 (27857ec) now │
│ ✓ accepted by z6MknSL…StBU8Vi (you) now
└─ ✓ accepted by z6MknSL…StBU8Vi (you) now │
╰─────────────────────────────────────────────────────────────────────╯
```

View File

@ -60,9 +60,12 @@ To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkE
* [new branch] patch/3581e83 -> patches/3581e83ad18f5cdd806ab50fa11cfd5dd4e8ae1c
```
Great, all fixed up, lets merge the code.
Great, all fixed up, lets accept and merge the code.
```
$ rad patch review 3581e83 --revision abb0360 --accept
✓ Patch 3581e83 accepted
✓ Synced with 1 node(s)
$ git checkout master
Your branch is up to date with 'rad/master'.
$ git merge patch/3581e83
@ -103,7 +106,8 @@ $ rad patch show 3581e83
│ ● opened by bob z6Mkt67…v4N1tRk now │
│ ↑ updated to 6de8527cdf51f96e12649c7278efe1dccfdee885 (27857ec) now │
│ * revised by alice (you) in abb0360 (f567f69) now │
│ ✓ merged by alice (you) at revision abb0360 (f567f69) now │
│ └─ ✓ accepted by alice (you) now │
│ └─ ✓ merged by alice (you) at revision abb0360 (f567f69) now │
╰─────────────────────────────────────────────────────────────────────╯
```

View File

@ -1,18 +1,21 @@
use std::collections::BTreeSet;
use std::iter;
use radicle::cob;
use radicle::cob::patch;
use radicle::cob::patch::{Patch, PatchId, Patches, Verdict};
use radicle::git;
use radicle::patch::{Merge, Review, Revision, RevisionId};
use radicle::prelude::*;
use radicle::profile::Profile;
use radicle::storage::git::Repository;
use radicle::storage::RemoteRepository as _;
use crate::terminal as term;
use term::format::Author;
use term::table::{Table, TableOptions};
use term::Element as _;
use super::common;
use crate::terminal as term;
/// List patches.
pub fn run(
@ -126,111 +129,314 @@ pub fn row(
])
}
pub fn timeline(
profile: &Profile,
patch: &Patch,
repository: &Repository,
) -> anyhow::Result<Vec<term::Line>> {
let open = term::Line::spaced([
term::format::positive("").into(),
term::format::default("opened by").into(),
])
.space()
.extend(Author::new(patch.author().id(), profile).line());
pub fn timeline<'a>(
profile: &'a Profile,
patch: &'a Patch,
) -> impl Iterator<Item = term::Line> + 'a {
Timeline::build(profile, patch).into_lines(profile)
}
let mut timeline = vec![(patch.timestamp(), open)];
let (root, _) = patch.root();
/// The timeline of a [`Patch`].
///
/// A `Patch` will always have opened with a root revision and may
/// have a series of revisions that update the patch.
///
/// The function, [`timeline`], builds a `Timeline` and converts it
/// into a series of [`term::Line`]s.
struct Timeline<'a> {
opened: Opened<'a>,
revisions: Vec<RevisionEntry<'a>>,
}
for (revision_id, revision) in patch.revisions() {
// Don't show an "update" line for the first revision.
if revision_id != root {
if revision.author() == patch.author() {
timeline.push((
impl<'a> Timeline<'a> {
fn build(profile: &Profile, patch: &'a Patch) -> Self {
let opened = Opened::from_patch(patch, profile);
let mut revisions = patch
.revisions()
.skip(1) // skip the root revision since it's handled in `Opened::from_patch`
.map(|(id, revision)| {
(
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.head(),
)))
.into(),
]
.into_iter(),
),
));
RevisionEntry::from_revision(patch, id, revision, profile),
)
})
.collect::<Vec<_>>();
revisions.sort_by_key(|(t, _)| *t);
Timeline {
opened,
revisions: revisions.into_iter().map(|(_, e)| e).collect(),
}
}
fn into_lines(self, profile: &'a Profile) -> impl Iterator<Item = term::Line> + 'a {
self.opened.into_lines(profile).chain(
self.revisions
.into_iter()
.flat_map(|r| r.into_lines(profile)),
)
}
}
/// The root `Revision` of the `Patch`.
struct Opened<'a> {
/// The `Author` of the patch.
author: Author<'a>,
/// When the patch was created.
timestamp: cob::Timestamp,
/// Any updates performed on the root `Revision`.
updates: Vec<Update<'a>>,
}
impl<'a> Opened<'a> {
fn from_patch(patch: &'a Patch, profile: &Profile) -> Self {
let (root, revision) = patch.root();
let mut updates = Vec::new();
updates.extend(revision.reviews().map(|(_, review)| {
(
review.timestamp(),
Update::Reviewed {
review: review.clone(),
},
)
}));
updates.extend(patch.merges().filter_map(|(_, merge)| {
if merge.revision == root {
Some((
merge.timestamp,
Update::Merged {
author: Author::new(&revision.author().id, profile),
merge: merge.clone(),
},
))
} else {
let (alias, nid) = Author::new(revision.author().id(), profile).labels();
timeline.push((
revision.timestamp(),
term::Line::spaced(
[
term::format::tertiary("*").into(),
term::format::default("revised by").into(),
alias.clone(),
nid.clone(),
term::format::default("in").into(),
term::format::dim(term::format::oid(revision_id)).into(),
term::format::parens(term::format::secondary(term::format::oid(
revision.head(),
)))
.into(),
]
.into_iter(),
),
));
None
}
}));
updates.sort_by_key(|(t, _)| *t);
Opened {
author: Author::new(&patch.author().id, profile),
timestamp: patch.timestamp(),
updates: updates.into_iter().map(|(_, up)| up).collect(),
}
}
for (nid, merge) in patch.merges().filter(|(_, m)| m.revision == revision_id) {
let peer = repository.remote(nid)?;
let (alias, nid) = Author::new(&peer.id, profile).labels();
let line = term::Line::spaced([
term::format::primary("").bold().into(),
term::format::default("merged by").into(),
alias,
nid,
term::format::default("at revision").into(),
term::format::dim(term::format::oid(merge.revision)).into(),
term::format::parens(term::format::secondary(term::format::oid(merge.commit)))
.into(),
]);
timeline.push((merge.timestamp, line));
}
for (reviewer, review) in revision.reviews() {
let verdict = review.verdict();
let verdict_symbol = match verdict {
Some(Verdict::Accept) => term::format::positive(""),
Some(Verdict::Reject) => term::format::negative(""),
None => term::format::dim(""),
};
let verdict_verb = match verdict {
Some(Verdict::Accept) => term::format::default("accepted"),
Some(Verdict::Reject) => term::format::default("rejected"),
None => term::format::default("reviewed"),
};
let peer = repository.remote(reviewer)?;
let line = term::Line::spaced([
verdict_symbol.into(),
verdict_verb.into(),
term::format::default("by").into(),
fn into_lines(self, profile: &'a Profile) -> impl Iterator<Item = term::Line> + 'a {
iter::once(
term::Line::spaced([
term::format::positive("").into(),
term::format::default("opened by").into(),
])
.space()
.extend(Author::new(&peer.id, profile).line());
.extend(self.author.line())
.space()
.extend([term::format::dim(term::format::timestamp(self.timestamp)).into()]),
)
.chain(self.updates.into_iter().map(|up| {
term::Line::spaced([term::Label::space(), term::Label::from("└─ ")])
.extend(up.into_line(profile))
}))
}
}
timeline.push((review.timestamp(), line));
/// A revision entry in the [`Timeline`].
enum RevisionEntry<'a> {
/// An `Updated` entry means that the original author of the
/// `Patch` created a new revision.
Updated {
/// When the `Revision` was created.
timestamp: cob::Timestamp,
/// The id of the `Revision`.
id: RevisionId,
/// The commit head of the `Revision`.
head: git::Oid,
/// All [`Update`]s that occurred on the `Revision`.
updates: Vec<Update<'a>>,
},
/// A `Revised` entry means that an author other than the original
/// author of the `Patch` created a new revision.
Revised {
/// The `Author` that created the `Revision` (that is not the
/// `Patch` author).
author: Author<'a>,
/// When the `Revision` was created.
timestamp: cob::Timestamp,
/// The id of the `Revision`.
id: RevisionId,
/// The commit head of the `Revision`.
head: git::Oid,
/// All [`Update`]s that occurred on the `Revision`.
updates: Vec<Update<'a>>,
},
}
impl<'a> RevisionEntry<'a> {
fn from_revision(
patch: &Patch,
id: RevisionId,
revision: &'a Revision,
profile: &Profile,
) -> Self {
let mut updates = Vec::new();
updates.extend(revision.reviews().map(|(_, review)| {
(
review.timestamp(),
Update::Reviewed {
review: review.clone(),
},
)
}));
updates.extend(patch.merges().filter_map(|(_, merge)| {
if merge.revision == id {
Some((
merge.timestamp,
Update::Merged {
author: Author::new(&revision.author().id, profile),
merge: merge.clone(),
},
))
} else {
None
}
}));
updates.sort_by_key(|(t, _)| *t);
if revision.author() == patch.author() {
RevisionEntry::Updated {
timestamp: revision.timestamp(),
id,
head: revision.head(),
updates: updates.into_iter().map(|(_, up)| up).collect(),
}
} else {
RevisionEntry::Revised {
author: Author::new(&revision.author().id, profile),
timestamp: revision.timestamp(),
id,
head: revision.head(),
updates: updates.into_iter().map(|(_, up)| up).collect(),
}
}
}
timeline.sort_by_key(|(t, _)| *t);
let mut lines = Vec::new();
for (time, mut line) in timeline.into_iter() {
line.push(term::Label::space());
line.push(term::format::dim(term::format::timestamp(time)));
lines.push(line);
fn into_lines(self, profile: &'a Profile) -> Vec<term::Line> {
match self {
RevisionEntry::Updated {
timestamp,
id,
head,
updates,
} => Self::updated(profile, timestamp, id, head, updates).collect(),
RevisionEntry::Revised {
author,
timestamp,
id,
head,
updates,
} => Self::revised(profile, author, timestamp, id, head, updates).collect(),
}
}
Ok(lines)
fn updated(
profile: &'a Profile,
timestamp: cob::Timestamp,
id: RevisionId,
head: git::Oid,
updates: Vec<Update<'a>>,
) -> impl Iterator<Item = term::Line> + 'a {
iter::once(term::Line::spaced([
term::format::tertiary("").into(),
term::format::default("updated to").into(),
term::format::dim(id).into(),
term::format::parens(term::format::secondary(term::format::oid(head))).into(),
term::format::dim(term::format::timestamp(timestamp)).into(),
]))
.chain(updates.into_iter().map(|up| {
term::Line::spaced([term::Label::space(), term::Label::from("└─ ")])
.extend(up.into_line(profile))
}))
}
fn revised(
profile: &'a Profile,
author: Author<'a>,
timestamp: cob::Timestamp,
id: RevisionId,
head: git::Oid,
updates: Vec<Update<'a>>,
) -> impl Iterator<Item = term::Line> + 'a {
let (alias, nid) = author.labels();
iter::once(term::Line::spaced([
term::format::tertiary("*").into(),
term::format::default("revised by").into(),
alias,
nid,
term::format::default("in").into(),
term::format::dim(term::format::oid(id)).into(),
term::format::parens(term::format::secondary(term::format::oid(head))).into(),
term::format::dim(term::format::timestamp(timestamp)).into(),
]))
.chain(updates.into_iter().map(|up| {
term::Line::spaced([term::Label::space(), term::Label::from("└─ ")])
.extend(up.into_line(profile))
}))
}
}
/// An update in the [`Patch`]'s timeline.
enum Update<'a> {
/// A revision of the patch was reviewed.
Reviewed { review: Review },
/// A revision of the patch was merged.
Merged { author: Author<'a>, merge: Merge },
}
impl<'a> Update<'a> {
fn timestamp(&self) -> cob::Timestamp {
match self {
Update::Reviewed { review } => review.timestamp(),
Update::Merged { merge, .. } => merge.timestamp,
}
}
fn into_line(self, profile: &Profile) -> term::Line {
let timestamp = self.timestamp();
let mut line = match self {
Update::Reviewed { review } => {
let verdict = review.verdict();
let verdict_symbol = match verdict {
Some(Verdict::Accept) => term::format::positive(""),
Some(Verdict::Reject) => term::format::negative(""),
None => term::format::dim(""),
};
let verdict_verb = match verdict {
Some(Verdict::Accept) => term::format::default("accepted"),
Some(Verdict::Reject) => term::format::default("rejected"),
None => term::format::default("reviewed"),
};
term::Line::spaced([
verdict_symbol.into(),
verdict_verb.into(),
term::format::default("by").into(),
])
.space()
.extend(Author::new(&review.author().id.into(), profile).line())
}
Update::Merged { author, merge } => {
let (alias, nid) = author.labels();
term::Line::spaced([
term::format::primary("").bold().into(),
term::format::default("merged by").into(),
alias,
nid,
term::format::default("at revision").into(),
term::format::dim(term::format::oid(merge.revision)).into(),
term::format::parens(term::format::secondary(term::format::oid(merge.commit)))
.into(),
])
}
};
line.push(term::Label::space());
line.push(term::format::dim(term::format::timestamp(timestamp)));
line
}
}

View File

@ -152,7 +152,7 @@ pub fn run(
.children(commits.into_iter().map(|l| l.boxed()))
.divider();
for line in list::timeline(profile, &patch, stored)? {
for line in list::timeline(profile, &patch) {
widget.push(line);
}
widget.print();

View File

@ -1490,6 +1490,11 @@ impl Review {
}
}
/// Review author.
pub fn author(&self) -> &Author {
&self.author
}
/// Review verdict.
pub fn verdict(&self) -> Option<Verdict> {
self.verdict