From 2dea54c6eeb7bf3a177fac77ae04fd4aeadd772d Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Mon, 18 Dec 2023 13:28:17 +0000 Subject: [PATCH] cob: mandatory `Oid` in `CodeLocation` To make `CodeLocation` reusable across different components, require that a commit's `Oid` is part of the struct. An `Oid` is necessary since the `path` can refer to a path at any point in time throughout the history of the repository. By pinning the commit that is being referred to, the path will refer to the path within the associated tree [of the commit]. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-httpd/src/api/json.rs | 2 +- radicle-httpd/src/api/v1/projects.rs | 4 +- radicle/src/cob/common.rs | 59 +++++++++++++++++++++++++++- radicle/src/cob/patch.rs | 58 ++------------------------- radicle/src/logger.rs | 2 +- 5 files changed, 66 insertions(+), 59 deletions(-) diff --git a/radicle-httpd/src/api/json.rs b/radicle-httpd/src/api/json.rs index 0e829dae..94bd491a 100644 --- a/radicle-httpd/src/api/json.rs +++ b/radicle-httpd/src/api/json.rs @@ -4,7 +4,7 @@ use std::path::Path; use std::str; use base64::prelude::{Engine, BASE64_STANDARD}; -use radicle::patch::CodeLocation; +use radicle::cob::CodeLocation; use serde_json::{json, Value}; use radicle::cob::issue::{Issue, IssueId}; diff --git a/radicle-httpd/src/api/v1/projects.rs b/radicle-httpd/src/api/v1/projects.rs index 79ac4025..d959a23b 100644 --- a/radicle-httpd/src/api/v1/projects.rs +++ b/radicle-httpd/src/api/v1/projects.rs @@ -3111,6 +3111,7 @@ mod routes { } ], "location": { + "commit": HEAD, "path": "README.md", "new": { "type": "lines", @@ -3224,7 +3225,7 @@ mod routes { "summary": "A small review", "comments": [ { - "id": "b108acfd2117480fd87012a5ab7cb69a0651d933", + "id": "2ed1c517997d3f1a83c6830ebab069c6e2ee967e", "author": { "id": "did:key:z6Mkk7oqY4pPxhMmGEotDYsFo97vhCj85BLY1H256HrJmjN8", }, @@ -3272,6 +3273,7 @@ mod routes { "timestamp": 1671125284, "replyTo": null, "location": { + "commit": HEAD, "path": "README.md", "old": null, "new": { diff --git a/radicle/src/cob/common.rs b/radicle/src/cob/common.rs index fc3de235..1518cf4d 100644 --- a/radicle/src/cob/common.rs +++ b/radicle/src/cob/common.rs @@ -1,12 +1,13 @@ use std::fmt; use std::fmt::Display; -use std::ops::Deref; +use std::ops::{Deref, Range}; +use std::path::PathBuf; use std::str::FromStr; use localtime::LocalTime; use serde::{Deserialize, Serialize}; -use crate::git_ext::Oid; +use crate::git::Oid; use crate::prelude::{Did, PublicKey}; /// Timestamp used for COB operations. @@ -385,3 +386,57 @@ mod test { Color::from_str("#abc").unwrap_err(); } } + +/// Describes a code location that can be used for comments on +/// patches, issues, and diffs. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CodeLocation { + /// [`Oid`] of the Git commit. + pub commit: Oid, + /// Path of file. + pub path: PathBuf, + /// Line range on old file. `None` for added files. + pub old: Option, + /// Line range on new file. `None` for deleted files. + pub new: Option, +} + +/// Code range. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", tag = "type")] +pub enum CodeRange { + /// One or more lines. + Lines { range: Range }, + /// Character range within a line. + Chars { line: usize, range: Range }, +} + +impl std::cmp::PartialOrd for CodeRange { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl std::cmp::Ord for CodeRange { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + match (self, other) { + (CodeRange::Lines { .. }, CodeRange::Chars { .. }) => std::cmp::Ordering::Less, + (CodeRange::Chars { .. }, CodeRange::Lines { .. }) => std::cmp::Ordering::Greater, + + (CodeRange::Lines { range: a }, CodeRange::Lines { range: b }) => { + a.clone().cmp(b.clone()) + } + ( + CodeRange::Chars { + line: l1, + range: r1, + }, + CodeRange::Chars { + line: l2, + range: r2, + }, + ) => l1.cmp(l2).then(r1.clone().cmp(r2.clone())), + } + } +} diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 4e196ea9..a3c38be5 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -2,8 +2,6 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::fmt; use std::ops::Deref; -use std::ops::Range; -use std::path::PathBuf; use std::str::FromStr; use amplify::Wrapper; @@ -14,7 +12,7 @@ use storage::RepositoryError; use thiserror::Error; use crate::cob; -use crate::cob::common::{Author, Authorization, Label, Reaction, Timestamp}; +use crate::cob::common::{Author, Authorization, CodeLocation, Label, Reaction, Timestamp}; use crate::cob::store::Transaction; use crate::cob::store::{Cob, CobAction}; use crate::cob::thread; @@ -1408,57 +1406,6 @@ impl fmt::Display for Verdict { } } -/// Code range. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "camelCase", tag = "type")] -pub enum CodeRange { - /// One or more lines. - Lines { range: Range }, - /// Character range within a line. - Chars { line: usize, range: Range }, -} - -impl std::cmp::PartialOrd for CodeRange { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl std::cmp::Ord for CodeRange { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - match (self, other) { - (CodeRange::Lines { .. }, CodeRange::Chars { .. }) => std::cmp::Ordering::Less, - (CodeRange::Chars { .. }, CodeRange::Lines { .. }) => std::cmp::Ordering::Greater, - - (CodeRange::Lines { range: a }, CodeRange::Lines { range: b }) => { - a.clone().cmp(b.clone()) - } - ( - CodeRange::Chars { - line: l1, - range: r1, - }, - CodeRange::Chars { - line: l2, - range: r2, - }, - ) => l1.cmp(l2).then(r1.clone().cmp(r2.clone())), - } - } -} - -/// Code location, used for attaching comments to diffs. -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct CodeLocation { - /// Path of file. - pub path: PathBuf, - /// Line range on old file. `None` for added files. - pub old: Option, - /// Line range on new file. `None` for deleted files. - pub new: Option, -} - /// A patch review on a revision. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Review { @@ -2351,12 +2298,14 @@ where #[cfg(test)] #[allow(clippy::unwrap_used)] mod test { + use std::path::PathBuf; use std::str::FromStr; use std::vec; use pretty_assertions::assert_eq; use super::*; + use crate::cob::common::CodeRange; use crate::cob::test::Actor; use crate::crypto::test::signer::MockSigner; use crate::identity; @@ -2784,6 +2733,7 @@ mod test { let (rid, _) = patch.latest(); let location = CodeLocation { + commit: branch.oid, path: PathBuf::from_str("README").unwrap(), old: None, new: Some(CodeRange::Lines { range: 5..8 }), diff --git a/radicle/src/logger.rs b/radicle/src/logger.rs index fdee6ab2..80b0d2b9 100644 --- a/radicle/src/logger.rs +++ b/radicle/src/logger.rs @@ -1,6 +1,6 @@ //! Logging module. //! -//! For test logging see [`test`]. +//! For test logging see [`mod@test`]. #[cfg(feature = "test")] pub mod test;