refactor: project a comment by id alone
commit
c4657d1refactor: project a comment by id alone
git_comment::project took both the id and a loaded Comment, then reloaded the stored document anyway on the gc-fallback path; it now takes just the id and loads once, so one name refers to the comment and the fallback reads the already-loaded retained tree.
Assisted-by: Claude:claude-fable-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
crates/git-comment/src/lib.rs
@@ -227,37 +227,32 @@
/// still [`Projection::Current`], relocated to a new path or shifted lines,
/// outdated because the anchored region was edited, or gone with its file.
///
-/// Tries [`git_anchor::project`] first; if `comment.anchor.commit` has been
-/// garbage collected, falls back to [`git_anchor::project_from_context`]
-/// against the comment's retained `context` blob, read directly off `id`'s
-/// ref rather than recomputed — recomputing would need the very commit that
-/// is gone.
+/// Tries [`git_anchor::project`] first; if the comment's anchored commit has
+/// been garbage collected, falls back to [`git_anchor::project_from_context`]
+/// against the comment's retained `context` blob — recomputing the context
+/// would need the very commit that is gone.
///
/// ## Requirements
///
/// @relation(comments.projection)
-pub fn project(
- repo: &Path,
- id: &str,
- comment: &Comment,
- target: &str,
-) -> Result<Projection, git_anchor::Error> {
- match git_anchor::project(repo, &comment.anchor, target) {
+pub fn project(repo: &Path, id: &str, target: &str) -> Result<Projection, git_anchor::Error> {
+ let stored: StoredComment = git_store::Store::open(repo)
+ .and_then(|store| store.load_item(COMMENTS_NS, id))
+ .map_err(|error| git_anchor::Error::Object(error.to_string()))?
+ .ok_or_else(|| git_anchor::Error::Object(format!("{COMMENTS_NS}/{id} does not exist")))?;
+ match git_anchor::project(repo, &stored.anchor, target) {
Err(git_anchor::Error::AnchorCommitMissing(_)) => {
- let context = retained_context(repo, id)
+ let context = retained_context(repo, &stored)
.map_err(|error| git_anchor::Error::Object(error.to_string()))?;
- git_anchor::project_from_context(repo, &comment.anchor, target, &context)
+ git_anchor::project_from_context(repo, &stored.anchor, target, &context)
}
other => other,
}
}
-/// Read the `context` blob out of comment `id`'s retained tree (see
-/// [`StoredComment`]) directly off its ref, for [`project`]'s fallback path.
-fn retained_context(repo: &Path, id: &str) -> Result<String, git_store::Error> {
- let stored: StoredComment = git_store::Store::open(repo)?
- .load_item(COMMENTS_NS, id)?
- .ok_or_else(|| git_store::Error::Ref(format!("{COMMENTS_NS}/{id} does not exist")))?;
+/// Read the `context` blob out of `stored`'s retained tree (see
+/// [`StoredComment`]), for [`project`]'s fallback path.
+fn retained_context(repo: &Path, stored: &StoredComment) -> Result<String, git_store::Error> {
let odb = odb_at(repo)?;
let mut tree_buf = Vec::new();
let tree = odb
@@ -496,7 +491,7 @@
// context fallback `project` reaches for once the anchor commit is
// gone.
assert_eq!(
- project(repo_path, &id, &loaded, &replacement).unwrap(),
+ project(repo_path, &id, &replacement).unwrap(),
Projection::Relocated {
path: "file.txt".to_owned(),
lines: Some(LineRange { start: 3, end: 3 }),
@@ -532,7 +527,7 @@
"two\n"
);
assert_eq!(
- project(dir.path(), &id, &loaded, "HEAD").unwrap(),
+ project(dir.path(), &id, "HEAD").unwrap(),
Projection::Current
);
}
crates/git-ents/src/main.rs
@@ -1031,7 +1031,7 @@
/// One-line description of where `comment` (`id`'s document) sits on `rev`.
fn describe_projection(repo: &Path, id: &str, comment: &Comment, rev: &str) -> String {
- match git_comment::project(repo, id, comment, rev) {
+ match git_comment::project(repo, id, rev) {
Ok(Projection::Current) => location(&comment.anchor.path, comment.anchor.lines),
Ok(Projection::Relocated { path, lines }) => location(&path, lines),
Ok(Projection::Outdated { path }) => format!("{path} [outdated]"),
crates/git-ents-server/src/web/pages.rs
@@ -685,7 +685,7 @@
};
let mut out = Vec::new();
for (id, comment) in comments {
- let Ok(projection) = git_comment::project(&repo, &id, &comment, "HEAD") else {
+ let Ok(projection) = git_comment::project(&repo, &id, "HEAD") else {
continue;
};
let (landed, lines, outdated) = match projection {