Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
radicle: use diff options in Patch
Fintan Halpenny committed 9 months ago
commit b6aeff4f923eab68f828046161d0c95ff8d2c878
parent 55f000268fc95c670ce2ba6b78ccdab925c1ab4e
5 files changed +145 -21
modified crates/radicle-cli/examples/rad-cob-show.md
@@ -72,7 +72,7 @@ We can show the patch COB too.

```
$ rad cob show --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --type xyz.radicle.patch --object d1f7f869fde9fac19c1779c4c2e77e8361333f91
-
{"title":"Start drafting peace treaty","author":{"id":"did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi"},"state":{"status":"open"},"target":"delegates","labels":[],"merges":{},"revisions":{"d1f7f869fde9fac19c1779c4c2e77e8361333f91":{"id":"d1f7f869fde9fac19c1779c4c2e77e8361333f91","author":{"id":"did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi"},"description":[{"author":"z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi","timestamp":1671125284000,"body":"See details.","embeds":[]}],"base":"f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354","oid":"575ed68c716d6aae81ea6b718fd9ac66a8eae532","discussion":{"comments":{},"timeline":[]},"reviews":{},"timestamp":1671125284000,"resolves":[],"reactions":[]}},"assignees":[],"timeline":["d1f7f869fde9fac19c1779c4c2e77e8361333f91"],"reviews":{}}
+
{"title":"Start drafting peace treaty","author":{"id":"did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi"},"state":{"status":"open"},"target":"delegates","labels":[],"merges":{},"revisions":{"d1f7f869fde9fac19c1779c4c2e77e8361333f91":{"id":"d1f7f869fde9fac19c1779c4c2e77e8361333f91","author":{"id":"did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi"},"description":[{"author":"z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi","timestamp":1671125284000,"body":"See details.","embeds":[]}],"base":"f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354","oid":"575ed68c716d6aae81ea6b718fd9ac66a8eae532","discussion":{"comments":{},"timeline":[]},"reviews":{},"timestamp":1671125284000,"resolves":[],"reactions":[]}},"assignees":[],"timeline":["d1f7f869fde9fac19c1779c4c2e77e8361333f91"],"reviews":{},"diffOptions":{"algorithm":"Histogram","skipBinary":false,"contextLines":3,"interhunkLines":0,"find":{"exact_match":false,"renames":{"limit":200,"rename_threshold":50}}}}
```

Finally let's update the issue and see the output of `rad cob show` also changes.
modified crates/radicle-remote-helper/src/push.rs
@@ -505,6 +505,7 @@ where
            patch::MergeTarget::default(),
            base,
            head,
+
            None,
            &[],
            signer,
        )
@@ -515,6 +516,7 @@ where
            patch::MergeTarget::default(),
            base,
            head,
+
            None,
            &[],
            signer,
        )
modified crates/radicle/src/cob/patch.rs
@@ -234,6 +234,11 @@ pub enum Action {
        /// Review comments resolved by this revision.
        #[serde(default, skip_serializing_if = "BTreeSet::is_empty")]
        resolves: BTreeSet<(EntryId, CommentId)>,
+
        /// The diff options for the patch.
+
        ///
+
        /// **N.B**: Only relevant for the initial revision creation.
+
        #[serde(default, skip_serializing_if = "Option::is_none")]
+
        diff_options: Option<diff::Options>,
    },
    #[serde(rename = "revision.edit")]
    RevisionEdit {
@@ -428,11 +433,64 @@ pub struct Patch {
    pub(super) timeline: Vec<EntryId>,
    /// Reviews index. Keeps track of reviews for better performance.
    pub(super) reviews: BTreeMap<ReviewId, Option<(RevisionId, ActorId)>>,
+
    /// The options used to provide the diff of the [`Patch`].
+
    #[serde(default)]
+
    pub(super) diff_options: diff::Options,
+
}
+

+
/// Data to create a new [`Patch`].
+
///
+
/// A new request can be constructed with [`CreatePatchRequest::new`]. The
+
/// [`MergeTarget`] and [`diff::Options`] can be set for the request by using
+
/// [`CreatePatchRequest::with_target`] and
+
/// [`CreatePatchRequest::with_diff_options`], respectively.
+
///
+
/// To use the [`CreatePatchRequest`], see [`Patch::new`].
+
pub struct CreatePatchRequest {
+
    title: String,
+
    target: MergeTarget,
+
    id: RevisionId,
+
    revision: Revision,
+
    diff_options: diff::Options,
+
}
+

+
impl CreatePatchRequest {
+
    /// Construct a new `CreatePatch` using default for `target`and
+
    /// `diff_options`.
+
    pub fn new(title: String, id: RevisionId, revision: Revision) -> Self {
+
        Self {
+
            title,
+
            target: MergeTarget::default(),
+
            id,
+
            revision,
+
            diff_options: diff::Options::default(),
+
        }
+
    }
+

+
    /// Set the `target`.
+
    pub fn with_target(self, target: MergeTarget) -> Self {
+
        Self { target, ..self }
+
    }
+

+
    /// Set the `diff_options`.
+
    pub fn with_diff_options(self, diff_options: diff::Options) -> Self {
+
        Self {
+
            diff_options,
+
            ..self
+
        }
+
    }
}

impl Patch {
-
    /// Construct a new patch object from a revision.
-
    pub fn new(title: String, target: MergeTarget, (id, revision): (RevisionId, Revision)) -> Self {
+
    /// Construct a new patch object from a [`CreatePatchRequest`].
+
    pub fn new(create: CreatePatchRequest) -> Self {
+
        let CreatePatchRequest {
+
            title,
+
            target,
+
            id,
+
            revision,
+
            diff_options,
+
        } = create;
        Self {
            title,
            author: revision.author.clone(),
@@ -444,6 +502,7 @@ impl Patch {
            assignees: BTreeSet::default(),
            timeline: vec![id.into_inner()],
            reviews: BTreeMap::default(),
+
            diff_options,
        }
    }

@@ -845,6 +904,8 @@ impl Patch {
                base,
                oid,
                resolves,
+
                // ignored for new revisions
+
                diff_options: _,
            } => {
                debug_assert!(!self.revisions.contains_key(&entry));
                let id = RevisionId(entry);
@@ -1136,6 +1197,7 @@ impl store::Cob for Patch {
            base,
            oid,
            resolves,
+
            diff_options,
        }) = actions.next()
        else {
            return Err(Error::Init("the first action must be of type `revision`"));
@@ -1152,7 +1214,11 @@ impl store::Cob for Patch {
            op.timestamp,
            resolves,
        );
-
        let mut patch = Patch::new(title, target, (RevisionId(op.id), revision));
+

+
        let create = CreatePatchRequest::new(title, RevisionId(op.id), revision)
+
            .with_target(target)
+
            .with_diff_options(diff_options.unwrap_or_default());
+
        let mut patch = Patch::new(create);

        for action in actions {
            match patch.authorization(&action, &op.author, &doc)? {
@@ -1914,6 +1980,22 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {
        self.push(Action::Merge { revision, commit })
    }

+
    fn initial_revision(
+
        &mut self,
+
        description: impl ToString,
+
        base: impl Into<git::Oid>,
+
        oid: impl Into<git::Oid>,
+
        opts: Option<diff::Options>,
+
    ) -> Result<(), store::Error> {
+
        self.push(Action::Revision {
+
            description: description.to_string(),
+
            base: base.into(),
+
            oid: oid.into(),
+
            resolves: BTreeSet::new(),
+
            diff_options: opts,
+
        })
+
    }
+

    /// Update a patch with a new revision.
    pub fn revision(
        &mut self,
@@ -1926,6 +2008,7 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {
            base: base.into(),
            oid: oid.into(),
            resolves: BTreeSet::new(),
+
            diff_options: None,
        })
    }

@@ -2599,6 +2682,7 @@ where
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        opts: Option<diff::Options>,
        labels: &[Label],
        cache: &'g mut C,
        signer: &Device<G>,
@@ -2613,6 +2697,7 @@ where
            target,
            base,
            oid,
+
            opts,
            labels,
            Lifecycle::default(),
            cache,
@@ -2628,6 +2713,7 @@ where
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        opts: Option<diff::Options>,
        labels: &[Label],
        cache: &'g mut C,
        signer: &Device<G>,
@@ -2642,6 +2728,7 @@ where
            target,
            base,
            oid,
+
            opts,
            labels,
            Lifecycle::Draft,
            cache,
@@ -2676,6 +2763,7 @@ where
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        opts: Option<diff::Options>,
        labels: &[Label],
        state: Lifecycle,
        cache: &'g mut C,
@@ -2686,7 +2774,7 @@ where
        G: crypto::signature::Signer<crypto::Signature>,
    {
        let (id, patch) = Transaction::initial("Create patch", &mut self.raw, signer, |tx, _| {
-
            tx.revision(description, base, oid)?;
+
            tx.initial_revision(description, base, oid, opts)?;
            tx.edit(title, target)?;

            if !labels.is_empty() {
@@ -2862,6 +2950,7 @@ mod test {
                target,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -2902,6 +2991,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -2935,6 +3025,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -2966,6 +3057,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3018,6 +3110,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3058,6 +3151,7 @@ mod test {
                base,
                oid,
                resolves: Default::default(),
+
                diff_options: None,
            },
            Action::Edit {
                title: String::from("My patch"),
@@ -3069,6 +3163,7 @@ mod test {
            base,
            oid,
            resolves: Default::default(),
+
            diff_options: None,
        }]);
        let a3 = alice.op::<Patch>([Action::RevisionRedact {
            revision: RevisionId(a2.id()),
@@ -3109,6 +3204,7 @@ mod test {
                    base,
                    oid,
                    resolves: Default::default(),
+
                    diff_options: None,
                },
                Action::Edit {
                    title: String::from("Some patch"),
@@ -3124,6 +3220,7 @@ mod test {
                base,
                oid,
                resolves: Default::default(),
+
                diff_options: None,
            },
            &alice,
        );
@@ -3169,6 +3266,7 @@ mod test {
                base,
                oid,
                resolves: Default::default(),
+
                diff_options: None,
            },
            Action::Edit {
                title: String::from("My patch"),
@@ -3206,6 +3304,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3251,6 +3350,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3282,6 +3382,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3333,6 +3434,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3380,6 +3482,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3435,6 +3538,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3484,6 +3588,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
modified crates/radicle/src/cob/patch/cache.rs
@@ -11,11 +11,12 @@ use crate::cob::store;
use crate::cob::{Label, ObjectId, TypeName};
use crate::git;
use crate::node::device::Device;
+
use crate::node::NodeId;
use crate::prelude::RepoId;
use crate::storage::{HasRepoId, ReadRepository, RepositoryError, SignRepository, WriteRepository};

use super::{
-
    ByRevision, MergeTarget, NodeId, Patch, PatchCounts, PatchId, PatchMut, Revision, RevisionId,
+
    diff, ByRevision, MergeTarget, Patch, PatchCounts, PatchId, PatchMut, Revision, RevisionId,
    State, Status,
};

@@ -115,6 +116,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        opts: Option<diff::Options>,
        labels: &[Label],
        signer: &Device<G>,
    ) -> Result<PatchMut<'a, 'g, R, C>, super::Error>
@@ -129,6 +131,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
            target,
            base,
            oid,
+
            opts,
            labels,
            &mut self.cache,
            signer,
@@ -145,6 +148,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        opts: Option<diff::Options>,
        labels: &[Label],
        signer: &Device<G>,
    ) -> Result<PatchMut<'a, 'g, R, C>, super::Error>
@@ -159,6 +163,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
            target,
            base,
            oid,
+
            opts,
            labels,
            &mut self.cache,
            signer,
@@ -714,7 +719,8 @@ mod tests {
    use crate::cob::thread::{Comment, Thread};
    use crate::cob::Author;
    use crate::patch::{
-
        ByRevision, MergeTarget, Patch, PatchCounts, PatchId, Revision, RevisionId, State, Status,
+
        ByRevision, CreatePatchRequest, Patch, PatchCounts, PatchId, Revision, RevisionId, State,
+
        Status,
    };
    use crate::prelude::Did;
    use crate::profile::env;
@@ -767,13 +773,15 @@ mod tests {
        let mut cache = memory(repo);
        assert!(cache.is_empty().unwrap());

-
        let patch = Patch::new("Patch #1".to_string(), MergeTarget::Delegates, revision());
+
        let (id, rev) = revision();
+
        let patch = Patch::new(CreatePatchRequest::new("Patch #1".to_string(), id, rev));
        let id = ObjectId::from_str("47799cbab2eca047b6520b9fce805da42b49ecab").unwrap();
        cache.update(&cache.rid(), &id, &patch).unwrap();

+
        let (id, rev) = revision();
        let patch = Patch {
            state: State::Archived,
-
            ..Patch::new("Patch #2".to_string(), MergeTarget::Delegates, revision())
+
            ..Patch::new(CreatePatchRequest::new("Patch #2".to_string(), id, rev))
        };
        let id = ObjectId::from_str("ae981ded6ed2ed2cdba34c8603714782667f18a3").unwrap();
        cache.update(&cache.rid(), &id, &patch).unwrap();
@@ -803,16 +811,18 @@ mod tests {
            .collect::<BTreeSet<PatchId>>();

        for id in open_ids.iter() {
-
            let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision());
+
            let (rev_id, rev) = revision();
+
            let patch = Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev));
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
                .unwrap();
        }

        for id in draft_ids.iter() {
+
            let (rev_id, rev) = revision();
            let patch = Patch {
                state: State::Draft,
-
                ..Patch::new(id.to_string(), MergeTarget::Delegates, revision())
+
                ..Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev))
            };
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
@@ -820,9 +830,10 @@ mod tests {
        }

        for id in archived_ids.iter() {
+
            let (rev_id, rev) = revision();
            let patch = Patch {
                state: State::Archived,
-
                ..Patch::new(id.to_string(), MergeTarget::Delegates, revision())
+
                ..Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev))
            };
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
@@ -830,12 +841,13 @@ mod tests {
        }

        for id in merged_ids.iter() {
+
            let (rev_id, rev) = revision();
            let patch = Patch {
                state: State::Merged {
                    revision: arbitrary::oid().into(),
                    commit: arbitrary::oid(),
                },
-
                ..Patch::new(id.to_string(), MergeTarget::Delegates, revision())
+
                ..Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev))
            };
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
@@ -869,7 +881,8 @@ mod tests {
        let mut patches = Vec::with_capacity(ids.len());

        for id in ids.iter() {
-
            let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision());
+
            let (rev_id, rev) = revision();
+
            let patch = Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev));
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
                .unwrap();
@@ -897,11 +910,11 @@ mod tests {
            .iter()
            .next()
            .expect("at least one revision should have been created");
-
        let mut patch = Patch::new(
+
        let mut patch = Patch::new(CreatePatchRequest::new(
            patch_id.to_string(),
-
            MergeTarget::Delegates,
-
            (*rev_id, rev.clone()),
-
        );
+
            *rev_id,
+
            rev.clone(),
+
        ));
        let timeline = revisions.keys().copied().collect::<Vec<_>>();
        patch
            .timeline
@@ -937,7 +950,8 @@ mod tests {
        let mut patches = Vec::with_capacity(ids.len());

        for id in ids.iter() {
-
            let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision());
+
            let (rev_id, rev) = revision();
+
            let patch = Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev));
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
                .unwrap();
@@ -964,7 +978,8 @@ mod tests {
        let mut patches = Vec::with_capacity(ids.len());

        for id in ids.iter() {
-
            let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision());
+
            let (rev_id, rev) = revision();
+
            let patch = Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev));
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
                .unwrap();
@@ -990,7 +1005,8 @@ mod tests {
            .collect::<BTreeSet<PatchId>>();

        for id in ids.iter() {
-
            let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision());
+
            let (rev_id, rev) = revision();
+
            let patch = Patch::new(CreatePatchRequest::new(id.to_string(), rev_id, rev));
            cache
                .update(&cache.rid(), &PatchId::from(*id), &patch)
                .unwrap();
modified crates/radicle/src/cob/test.rs
@@ -237,6 +237,7 @@ impl<G: Signer> Actor<G> {
                    base,
                    oid,
                    resolves: Default::default(),
+
                    diff_options: None,
                },
                patch::Action::Edit {
                    title: title.to_string(),