From 19d341d32ac5dc07b7b75c1bad15b11b98cfa2ba Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Mon, 2 Jan 2023 16:24:00 -0800 Subject: [PATCH] Templater: naive implementation of shortest prefix highlight for ids This creates a templater function `short_underscore_prefix` for commit and change ids. It is similar to `short` function, but shows one fewer hexadecimal digit and inserts an underscore after the shortest unique prefix. Highlighting with an underline and perhaps color/bold will be in a follow-up PR. The implementation is quadratic, a simple comparison of each id with every other id. It is replaced in a subsequent commit. The problem with it is that, while it works fine for a `jj`-sized repo, it becomes is painfully slow with a repo the size of git/git. Still, this naive implemenation is included here since it's simple, and could be used as a reference implementation. The `shortest_unique_prefix_length` function goes into `repo.rs` since that's convenient for follow-up commits in this PR to have nicer diffs. --- lib/src/repo.rs | 37 ++++++++++++- src/template_parser.rs | 14 +++-- src/templater.rs | 29 +++++++++++ tests/test_log_command.rs | 107 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 181 insertions(+), 6 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 41f979e19..a8493b750 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -24,7 +24,7 @@ use once_cell::sync::OnceCell; use thiserror::Error; use self::dirty_cell::DirtyCell; -use crate::backend::{Backend, BackendError, BackendResult, ChangeId, CommitId, TreeId}; +use crate::backend::{Backend, BackendError, BackendResult, ChangeId, CommitId, ObjectId, TreeId}; use crate::commit::Commit; use crate::commit_builder::CommitBuilder; use crate::dag_walk::topo_order_reverse; @@ -37,6 +37,7 @@ use crate::op_store::{ BranchTarget, OpStore, OperationId, OperationMetadata, RefTarget, WorkspaceId, }; use crate::operation::Operation; +use crate::revset::RevsetExpression; use crate::rewrite::DescendantRebaser; use crate::settings::{RepoSettings, UserSettings}; use crate::simple_op_heads_store::SimpleOpHeadsStore; @@ -241,6 +242,40 @@ impl ReadonlyRepo { }) } + // An interface for testing this functionality directly is constructed in + // a follow-up commit. + pub fn shortest_unique_prefix_length(&self, target_id_hex: &str) -> usize { + let all_visible_revisions = RevsetExpression::all() + .evaluate(self.as_repo_ref(), None) + .unwrap(); + let change_hex_iter = all_visible_revisions + .iter() + .map(|index_entry| index_entry.change_id().hex()); + // We need to account for rewritten commits as well + let index = self.as_repo_ref().index(); + let commit_hex_iter = index + .iter() + .map(|index_entry| index_entry.commit_id().hex()); + + let target_id_hex = target_id_hex.as_bytes(); + itertools::chain(change_hex_iter, commit_hex_iter) + .filter_map(|id_hex| { + let id_hex = id_hex.as_bytes(); + let common_len = target_id_hex + .iter() + .zip(id_hex.iter()) + .take_while(|(a, b)| a == b) + .count(); + if common_len == target_id_hex.len() && common_len == id_hex.len() { + None // Target id matched itself + } else { + Some(common_len + 1) + } + }) + .max() + .unwrap_or(0) + } + pub fn store(&self) -> &Arc { &self.store } diff --git a/src/template_parser.rs b/src/template_parser.rs index e231548d5..3d49de27e 100644 --- a/src/template_parser.rs +++ b/src/template_parser.rs @@ -23,11 +23,12 @@ use pest_derive::Parser; use crate::formatter::PlainTextFormatter; use crate::templater::{ AuthorProperty, BranchProperty, CommitOrChangeId, CommitOrChangeIdKeyword, - CommitOrChangeIdShort, CommitterProperty, ConditionalTemplate, ConflictProperty, - ConstantTemplateProperty, DescriptionProperty, DivergentProperty, DynamicLabelTemplate, - EmptyProperty, GitRefsProperty, IsGitHeadProperty, IsWorkingCopyProperty, LabelTemplate, - ListTemplate, LiteralTemplate, SignatureTimestamp, StringPropertyTemplate, TagProperty, - Template, TemplateFunction, TemplateProperty, WorkingCopiesProperty, + CommitOrChangeIdShort, CommitOrChangeIdShortPrefixAndBrackets, CommitterProperty, + ConditionalTemplate, ConflictProperty, ConstantTemplateProperty, DescriptionProperty, + DivergentProperty, DynamicLabelTemplate, EmptyProperty, GitRefsProperty, IsGitHeadProperty, + IsWorkingCopyProperty, LabelTemplate, ListTemplate, LiteralTemplate, SignatureTimestamp, + StringPropertyTemplate, TagProperty, Template, TemplateFunction, TemplateProperty, + WorkingCopiesProperty, }; use crate::time_util; @@ -215,6 +216,9 @@ fn parse_commit_or_chain_id_method<'a>( let this_function = match name.as_str() { "short" => Property::String(Box::new(CommitOrChangeIdShort { repo })), + "short_prefix_and_brackets" => { + Property::String(Box::new(CommitOrChangeIdShortPrefixAndBrackets { repo })) + } name => panic!("no such commit ID method: {name}"), }; let chain_method = inner.last().unwrap(); diff --git a/src/templater.rs b/src/templater.rs index 29713c3d7..122e48c1b 100644 --- a/src/templater.rs +++ b/src/templater.rs @@ -444,6 +444,25 @@ impl CommitOrChangeIdKeyword { pub fn short_format(commit_or_change_id: CommitOrChangeId) -> String { commit_or_change_id.hex()[..12].to_string() } + pub fn short_prefix_and_brackets_format( + commit_or_change_id: CommitOrChangeId, + repo: RepoRef, + ) -> String { + highlight_shortest_prefix(&commit_or_change_id.hex(), 12, repo) + } +} + +fn highlight_shortest_prefix(hex: &str, total_len: usize, repo: RepoRef) -> String { + let prefix_len = repo.base_repo().shortest_unique_prefix_length(hex); + if prefix_len < total_len - 1 { + format!( + "{}[{}]", + &hex[0..prefix_len], + &hex[prefix_len..total_len - 1] + ) + } else { + hex[0..total_len].to_string() + } } pub struct CommitOrChangeIdShort<'a> { @@ -456,6 +475,16 @@ impl TemplateProperty for CommitOrChangeIdShort<'_> { } } +pub struct CommitOrChangeIdShortPrefixAndBrackets<'a> { + pub repo: RepoRef<'a>, +} + +impl TemplateProperty for CommitOrChangeIdShortPrefixAndBrackets<'_> { + fn extract(&self, context: &CommitOrChangeId) -> String { + CommitOrChangeIdKeyword::short_prefix_and_brackets_format(context.clone(), self.repo) + } +} + impl TemplateProperty for CommitOrChangeIdKeyword { fn extract(&self, context: &Commit) -> CommitOrChangeId { match &self.0 { diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs index ecd94781d..3cf38db86 100644 --- a/tests/test_log_command.rs +++ b/tests/test_log_command.rs @@ -287,6 +287,113 @@ fn test_log_with_or_without_diff() { "###); } +#[test] +fn test_log_prefix_highlight() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + let prefix_format = r#" + "Change " change_id.short_prefix_and_brackets() " " description.first_line() + " " commit_id.short_prefix_and_brackets() " " branches + "#; + + std::fs::write(repo_path.join("file"), "original file\n").unwrap(); + test_env.jj_cmd_success(&repo_path, &["describe", "-m", "initial"]); + test_env.jj_cmd_success(&repo_path, &["branch", "c", "original"]); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["log", "-r", "original", "-T", prefix_format]), + @r###" + @ Change 9[a45c67d3e9] initial b[a1a30916d2] original + ~ + "### + ); + for i in 1..50 { + test_env.jj_cmd_success(&repo_path, &["new", "-m", &format!("commit{i}")]); + std::fs::write(repo_path.join("file"), format!("file {i}\n")).unwrap(); + } + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["log", "-r", "original", "-T", prefix_format]), + @r###" + o Change 9a4[5c67d3e9] initial ba1[a30916d2] original + ~ + "### + ); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["log", "-r", "@-----------..@", "-T", prefix_format]), + @r###" + @ Change 4c9[32da8013] commit49 d8[3437a2cef] + o Change 0d[58f15eaba] commit48 f3[abb4ea0ac] + o Change fc[e6c2c5912] commit47 38e[891bea27] + o Change d5[1defcac30] commit46 1c[04d947707] + o Change 4f[13b1391d6] commit45 747[24ae22b1] + o Change 6a[de2950a04] commit44 c7a[a67cf7bb] + o Change 06c[482e452d] commit43 8e[c99dfcb6c] + o Change 392[beeb018e] commit42 8f0[e60411b7] + o Change a1[b73d3ff91] commit41 71[d6937a66c] + o Change 708[8f461291] commit40 db[572049026] + o Change c49[f7f006c7] commit39 d94[54fec8a6] + ~ + "### + ); +} + +#[test] +fn test_log_prefix_highlight_counts_hidden_commits() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + let prefix_format = r#" + "Change " change_id.short_prefix_and_brackets() " " description.first_line() + " " commit_id.short_prefix_and_brackets() " " branches + "#; + + std::fs::write(repo_path.join("file"), "original file\n").unwrap(); + test_env.jj_cmd_success(&repo_path, &["describe", "-m", "initial"]); + test_env.jj_cmd_success(&repo_path, &["branch", "c", "original"]); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["log", "-r", "all()", "-T", prefix_format]), + @r###" + @ Change 9[a45c67d3e9] initial b[a1a30916d2] original + o Change 000000000000 (no description set) 000000000000 + "### + ); + for i in 1..100 { + test_env.jj_cmd_success(&repo_path, &["describe", "-m", &format!("commit{i}")]); + std::fs::write(repo_path.join("file"), format!("file {i}\n")).unwrap(); + } + // The first commit still exists. Its unique prefix became longer. + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["log", "-r", "ba1", "-T", prefix_format]), + @r###" + o Change 9a4[5c67d3e9] initial ba1[a30916d2] + ~ + "### + ); + // The first commit is no longer visible + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["log", "-r", "all()", "-T", prefix_format]), + @r###" + @ Change 9a4[5c67d3e9] commit99 de[3177d2acf] original + o Change 000000000000 (no description set) 000000000000 + "### + ); + insta::assert_snapshot!( + test_env.jj_cmd_failure(&repo_path, &["log", "-r", "d", "-T", prefix_format]), + @r###" + Error: Commit or change id prefix "d" is ambiguous + "### + ); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["log", "-r", "de", "-T", prefix_format]), + @r###" + @ Change 9a4[5c67d3e9] commit99 de[3177d2acf] original + ~ + "### + ); +} + #[test] fn test_log_divergence() { let test_env = TestEnvironment::default();