refactor: drop the stored snippet from Anchor and derive it at read time
commit
4b7842arefactor: drop the stored snippet from Anchor and derive it at read time
The anchored text is fully derivable from the content-addressed blob and the line range, so storing it froze a second copy into the on-disk format that could only ever agree or be a bug. capture still validates the range against the blob; display slices it on demand.
feat: add git_anchor::snippet() deriving the anchored text Assisted-by: Claude:claude-fable-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
crates/git-anchor/src/lib.rs
@@ -2,8 +2,10 @@
//!
//! An [`Anchor`] records exactly where in a repository something (a comment, a
//! review note) was attached: the commit it was written against, the path and
-//! blob at that commit, an optional line range, and the anchored text itself.
-//! The anchor is authoritative at creation and never mutated. [`project`]
+//! blob at that commit, and an optional line range. The anchored text is never
+//! stored — the blob is content-addressed, so [`snippet`] derives it exactly
+//! at read time. The anchor is authoritative at creation and never mutated.
+//! [`project`]
//! answers, at read time, where that position sits on any *other* commit —
//! following renames through git's rewrite tracking and shifting line ranges
//! through the blob's diff hunks, the way git itself re-derives positions when
@@ -83,9 +85,6 @@
pub blob: String,
/// The anchored lines, or `None` for a whole-file anchor.
pub lines: Option<LineRange>,
- /// The exact text of the anchored lines (empty for a whole-file anchor),
- /// kept for display and for fuzzy re-matching as a later enhancement.
- pub snippet: String,
}
/// Where an [`Anchor`] sits on a target commit, as computed by [`project`].
@@ -114,9 +113,9 @@
}
/// Build the [`Anchor`] for `path` (and optionally `lines`) as it exists at
-/// `revision` in `repo`, resolving the revision to a full commit id, recording
-/// the file's blob id, and capturing the anchored lines' exact text. Fails
-/// when the path is not a file at that commit or the range does not fit it.
+/// `revision` in `repo`, resolving the revision to a full commit id and
+/// recording the file's blob id. Fails when the path is not a file at that
+/// commit or the range does not fit it.
pub fn capture(
repo: &Path,
revision: &str,
@@ -138,37 +137,53 @@
path: path.to_owned(),
})?;
let blob = entry.object_id();
- let snippet = match lines {
- None => String::new(),
- Some(range) => {
- let data = read_blob(&repo, blob)?;
- let all: Vec<&[u8]> = data.lines_with_terminator().collect();
- let out_of_range = || Error::LinesOutOfRange {
- path: path.to_owned(),
- start: range.start,
- end: range.end,
- len: u64::try_from(all.len()).unwrap_or(u64::MAX),
- };
- // One slice lookup validates the whole range: start == 0 dies in
- // checked_sub, an inverted or oversized range dies in get.
- let first = usize::try_from(range.start)
- .ok()
- .and_then(|start| start.checked_sub(1))
- .ok_or_else(out_of_range)?;
- let last = usize::try_from(range.end).ok().ok_or_else(out_of_range)?;
- let bytes = all.get(first..last).ok_or_else(out_of_range)?.concat();
- String::from_utf8_lossy(&bytes).into_owned()
- }
- };
+ if let Some(range) = lines {
+ let data = read_blob(&repo, blob)?;
+ lines_of(&data, path, range)?;
+ }
Ok(Anchor {
commit: commit_id,
path: path.to_owned(),
blob: blob.to_string(),
lines,
- snippet,
})
}
+/// The exact text of `anchor`'s lines — the whole file for a whole-file
+/// anchor — derived at read time from the content-addressed blob the anchor
+/// names, so it can never disagree with what was anchored.
+pub fn snippet(repo: &Path, anchor: &Anchor) -> Result<String, Error> {
+ let repo = gix::open(repo).map_err(|error| Error::Open(Box::new(error)))?;
+ let blob = ObjectId::from_hex(anchor.blob.as_bytes())
+ .map_err(|_error| Error::Resolve(anchor.blob.clone()))?;
+ let data = read_blob(&repo, blob)?;
+ match anchor.lines {
+ None => Ok(String::from_utf8_lossy(&data).into_owned()),
+ Some(range) => lines_of(&data, &anchor.path, range),
+ }
+}
+
+/// The text of the 1-based inclusive `range` within `data`, or
+/// [`Error::LinesOutOfRange`] (naming `path`) when the range does not fit.
+fn lines_of(data: &[u8], path: &str, range: LineRange) -> Result<String, Error> {
+ let all: Vec<&[u8]> = data.lines_with_terminator().collect();
+ let out_of_range = || Error::LinesOutOfRange {
+ path: path.to_owned(),
+ start: range.start,
+ end: range.end,
+ len: u64::try_from(all.len()).unwrap_or(u64::MAX),
+ };
+ // One slice lookup validates the whole range: start == 0 dies in
+ // checked_sub, an inverted or oversized range dies in get.
+ let first = usize::try_from(range.start)
+ .ok()
+ .and_then(|start| start.checked_sub(1))
+ .ok_or_else(out_of_range)?;
+ let last = usize::try_from(range.end).ok().ok_or_else(out_of_range)?;
+ let bytes = all.get(first..last).ok_or_else(out_of_range)?.concat();
+ Ok(String::from_utf8_lossy(&bytes).into_owned())
+}
+
/// Project `anchor` onto `target` (a revision in `repo`): the fast path
/// returns [`Projection::Current`] when the target tree holds the anchor's
/// blob at its path; otherwise the anchor commit's tree is diffed against the
@@ -412,7 +427,7 @@
}
#[test]
- fn capture_records_the_commit_blob_and_snippet() {
+ fn capture_records_the_commit_and_blob_and_snippet_derives_the_text() {
let dir = repo();
std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap();
commit_all(dir.path(), "one");
@@ -420,9 +435,9 @@
let anchor = capture(dir.path(), "HEAD", "file.txt", range(3, 4)).unwrap();
assert_eq!(anchor.commit, head(dir.path()));
assert_eq!(anchor.path, "file.txt");
- assert_eq!(anchor.snippet, "line 3\nline 4\n");
assert_eq!(anchor.lines, range(3, 4));
assert!(!anchor.blob.is_empty());
+ assert_eq!(snippet(dir.path(), &anchor).unwrap(), "line 3\nline 4\n");
}
#[test]
@@ -559,7 +574,7 @@
std::fs::write(dir.path().join("file.txt"), numbered(1..=10)).unwrap();
commit_all(dir.path(), "one");
let anchor = capture(dir.path(), "HEAD", "file.txt", None).unwrap();
- assert_eq!(anchor.snippet, "");
+ assert_eq!(snippet(dir.path(), &anchor).unwrap(), numbered(1..=10));
let edited = numbered(1..=10).replace("line 5\n", "line five\n");
std::fs::write(dir.path().join("file.txt"), edited).unwrap();
crates/git-comment/src/lib.rs
@@ -134,7 +134,6 @@
path: "src/lib.rs".to_owned(),
blob: "89abcdef0123456789abcdef0123456789abcdef".to_owned(),
lines: Some(LineRange { start: 3, end: 4 }),
- snippet: "let x = 1;\nlet y = 2;\n".to_owned(),
},
issue: issue.map(str::to_owned),
}
@@ -248,7 +247,10 @@
store(dir.path(), &id, &written, AUTHOR).unwrap();
let loaded = load(dir.path(), &id).unwrap().unwrap();
- assert_eq!(loaded.anchor.snippet, "two\n");
+ assert_eq!(
+ git_anchor::snippet(dir.path(), &loaded.anchor).unwrap(),
+ "two\n"
+ );
assert_eq!(
project(dir.path(), &loaded, "HEAD").unwrap(),
Projection::Current
@@ -280,7 +282,7 @@
#[test]
fn loads_the_on_disk_comment_format() {
// A fixture written as the real on-disk layout — a `body` blob, an
- // `anchor/` subtree of `commit`/`path`/`blob`/`snippet` blobs with a
+ // `anchor/` subtree of `commit`/`path`/`blob` blobs with a
// `lines/some/{start,end}` Option subtree, and an `issue/some` Option
// blob — must keep loading, guarding the Comment document's shape
// against an incompatible change to data already on a ref.
@@ -311,12 +313,10 @@
"100644 blob {}\tcommit\n\
100644 blob {}\tpath\n\
100644 blob {}\tblob\n\
- 040000 tree {lines_tree}\tlines\n\
- 100644 blob {}\tsnippet\n",
+ 040000 tree {lines_tree}\tlines\n",
blob(&expected.anchor.commit),
blob(&expected.anchor.path),
blob(&expected.anchor.blob),
- blob(&expected.anchor.snippet),
),
);
let issue_tree = git_with_stdin(