annotate: add option to not search all ancestors of starting commit

The primary use case is to exclude immutable commits when calculating line
ranges to absorb. For example, "jj absorb" will build annotation of @ revision
with domain = mutable().
This commit is contained in:
Yuya Nishihara 2024-10-29 18:44:36 +09:00
parent 325402dc94
commit 85e0a8b068
3 changed files with 86 additions and 18 deletions

View file

@ -16,6 +16,7 @@ use jj_lib::annotate::get_annotation_for_file;
use jj_lib::annotate::FileAnnotation;
use jj_lib::commit::Commit;
use jj_lib::repo::Repo;
use jj_lib::revset::RevsetExpression;
use tracing::instrument;
use crate::cli_util::CommandHelper;
@ -69,7 +70,12 @@ pub(crate) fn cmd_file_annotate(
.get_string("templates.annotate_commit_summary")?;
let template = workspace_command.parse_commit_template(ui, &annotate_commit_summary_text)?;
let annotation = get_annotation_for_file(repo.as_ref(), &starting_commit, &file_path)?;
// TODO: Should we add an option to limit the domain to e.g. recent commits?
// Note that this is probably different from "--skip REVS", which won't
// exclude the revisions, but will ignore diffs in those revisions as if
// ancestor revisions had new content.
let domain = RevsetExpression::all();
let annotation = get_annotation_for_file(repo.as_ref(), &starting_commit, &domain, &file_path)?;
render_file_annotation(repo.as_ref(), ui, &template, &annotation)?;
Ok(())
@ -84,6 +90,7 @@ fn render_file_annotation(
ui.request_pager();
let mut formatter = ui.stdout_formatter();
for (line_no, (commit_id, line)) in annotation.lines().enumerate() {
let commit_id = commit_id.expect("should reached to the empty ancestor");
let commit = repo.store().get_commit(commit_id)?;
template_render.format(&commit, formatter.as_mut())?;
write!(formatter, " {:>4}: ", line_no + 1)?;

View file

@ -20,6 +20,7 @@
use std::collections::hash_map;
use std::collections::HashMap;
use std::rc::Rc;
use bstr::BStr;
use bstr::BString;
@ -40,6 +41,7 @@ use crate::graph::GraphEdgeType;
use crate::merged_tree::MergedTree;
use crate::repo::Repo;
use crate::repo_path::RepoPath;
use crate::revset::ResolvedRevsetExpression;
use crate::revset::RevsetEvaluationError;
use crate::revset::RevsetExpression;
use crate::revset::RevsetFilterPredicate;
@ -57,9 +59,9 @@ impl FileAnnotation {
///
/// For each line, the `commit_id` points to the originator commit of the
/// line. The `line` includes newline character.
pub fn lines(&self) -> impl Iterator<Item = (&CommitId, &BStr)> {
pub fn lines(&self) -> impl Iterator<Item = (Option<&CommitId>, &BStr)> {
itertools::zip_eq(&self.line_map, self.text.split_inclusive(|b| *b == b'\n'))
.map(|(commit_id, line)| (commit_id.as_ref().unwrap(), line.as_ref()))
.map(|(commit_id, line)| (commit_id.as_ref(), line.as_ref()))
}
/// File content at the starting commit.
@ -102,16 +104,22 @@ impl Source {
type OriginalLineMap = Vec<Option<CommitId>>;
/// Get line by line annotations for a specific file path in the repo.
///
/// The `domain` expression narrows the range of ancestors to search. It will be
/// intersected as `domain & ::starting_commit & files(file_path)`. The
/// `starting_commit` is assumed to be included in the `domain`.
///
/// If the file is not found, returns empty results.
pub fn get_annotation_for_file(
repo: &dyn Repo,
starting_commit: &Commit,
domain: &Rc<ResolvedRevsetExpression>,
file_path: &RepoPath,
) -> Result<FileAnnotation, RevsetEvaluationError> {
let mut source = Source::load(starting_commit, file_path)?;
source.fill_line_map();
let text = source.text.clone();
let line_map = process_commits(repo, starting_commit.id(), source, file_path)?;
let line_map = process_commits(repo, starting_commit.id(), source, domain, file_path)?;
Ok(FileAnnotation { line_map, text })
}
@ -122,17 +130,19 @@ fn process_commits(
repo: &dyn Repo,
starting_commit_id: &CommitId,
starting_source: Source,
domain: &Rc<ResolvedRevsetExpression>,
file_name: &RepoPath,
) -> Result<OriginalLineMap, RevsetEvaluationError> {
let predicate = RevsetFilterPredicate::File(FilesetExpression::file_path(file_name.to_owned()));
// TODO: If the domain isn't a contiguous range, changes masked out by it
// might not be caught by the closest ancestor revision. For example,
// domain=merges() would pick up almost nothing because merge revisions
// are usually empty. Perhaps, we want to query `files(file_path,
// within_sub_graph=domain)`, not `domain & files(file_path)`.
let ancestors = RevsetExpression::commit(starting_commit_id.clone()).ancestors();
let revset = RevsetExpression::commit(starting_commit_id.clone())
.union(
&RevsetExpression::commit(starting_commit_id.clone())
.ancestors()
.filtered(predicate),
)
.evaluate(repo)
.map_err(|e| e.expect_backend_error())?;
.union(&domain.intersection(&ancestors).filtered(predicate))
.evaluate(repo)?;
let mut original_line_map = vec![None; starting_source.line_map.len()];
let mut commit_source_map = HashMap::from([(starting_commit_id.clone(), starting_source)]);
@ -171,9 +181,6 @@ fn process_commit(
};
for parent_edge in edges {
if parent_edge.edge_type == GraphEdgeType::Missing {
continue;
}
let parent_commit_id = &parent_edge.target;
let parent_source = match commit_source_map.entry(parent_commit_id.clone()) {
hash_map::Entry::Occupied(entry) => entry.into_mut(),
@ -215,7 +222,10 @@ fn process_commit(
} else {
itertools::merge(parent_source.line_map.iter().copied(), new_parent_line_map).collect()
};
if parent_source.line_map.is_empty() {
// If an omitted parent had the file, leave these lines unresolved.
// TODO: These unresolved lines could be copied to the original_line_map
// as Err(commit_id) or something instead of None.
if parent_source.line_map.is_empty() || parent_edge.edge_type == GraphEdgeType::Missing {
commit_source_map.remove(parent_commit_id);
}
}

View file

@ -13,6 +13,7 @@
// limitations under the License.
use std::fmt::Write as _;
use std::rc::Rc;
use jj_lib::annotate::get_annotation_for_file;
use jj_lib::backend::CommitId;
@ -24,6 +25,8 @@ use jj_lib::commit::Commit;
use jj_lib::repo::MutableRepo;
use jj_lib::repo::Repo;
use jj_lib::repo_path::RepoPath;
use jj_lib::revset::ResolvedRevsetExpression;
use jj_lib::revset::RevsetExpression;
use jj_lib::settings::UserSettings;
use testutils::create_tree;
use testutils::TestRepo;
@ -54,11 +57,23 @@ fn create_commit_fn<'a>(
}
fn annotate(repo: &dyn Repo, commit: &Commit, file_path: &RepoPath) -> String {
let annotation = get_annotation_for_file(repo, commit, file_path).unwrap();
let domain = RevsetExpression::all();
annotate_within(repo, commit, &domain, file_path)
}
fn annotate_within(
repo: &dyn Repo,
commit: &Commit,
domain: &Rc<ResolvedRevsetExpression>,
file_path: &RepoPath,
) -> String {
let annotation = get_annotation_for_file(repo, commit, domain, file_path).unwrap();
let mut output = String::new();
for (commit_id, line) in annotation.lines() {
let commit = repo.store().get_commit(commit_id).unwrap();
let desc = commit.description().trim_end();
let commit = commit_id.map(|id| repo.store().get_commit(id).unwrap());
let desc = commit
.as_ref()
.map_or("*******", |commit| commit.description().trim_end());
write!(output, "{desc}: {line}").unwrap();
}
output
@ -139,6 +154,42 @@ fn test_annotate_merge_simple() {
commit1: 1
commit3: 3
"#);
// Exclude the fork commit and its ancestors.
let domain = RevsetExpression::commit(commit1.id().clone())
.ancestors()
.negated();
insta::assert_snapshot!(annotate_within(tx.repo(), &commit4, &domain, file_path), @r"
commit2: 2
*******: 1
commit3: 3
");
// Exclude one side of the merge and its ancestors.
let domain = RevsetExpression::commit(commit2.id().clone())
.ancestors()
.negated();
insta::assert_snapshot!(annotate_within(tx.repo(), &commit4, &domain, file_path), @r"
*******: 2
*******: 1
commit3: 3
");
// Exclude both sides of the merge and their ancestors.
let domain = RevsetExpression::commit(commit4.id().clone());
insta::assert_snapshot!(annotate_within(tx.repo(), &commit4, &domain, file_path), @r"
*******: 2
*******: 1
*******: 3
");
// Exclude intermediate commit, which is useless but works.
let domain = RevsetExpression::commit(commit3.id().clone()).negated();
insta::assert_snapshot!(annotate_within(tx.repo(), &commit4, &domain, file_path), @r"
commit2: 2
commit1: 1
commit4: 3
");
}
#[test]