From 7913f90869e0418a598d93c65d9955c28fcc1657 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 14 Feb 2023 22:48:49 +0900 Subject: [PATCH] templater: remove "brackets" short id style in favor of template alias Though .prefix() + .rest() has to call .shortest() twice, I don't think the added cost would be significant. --- CHANGELOG.md | 3 +- src/cli_util.rs | 1 - src/config-schema.json | 1 - src/template_parser.rs | 9 --- src/templater.rs | 10 ---- tests/test_commit_template.rs | 32 ----------- tests/test_log_command.rs | 100 +++++++++------------------------- 7 files changed, 28 insertions(+), 128 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b5c72f3f..77429388e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,8 +110,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj log` now highlights the shortest unique prefix of every commit and change id and shows the rest in gray. To disable, set the new `ui.unique-prefixes` - option to `none`. For a presentation that doesn't rely on color, set it to - `brackets`. + option to `none`. * It is now possible to change the lengths of the ids `jj log` prints with the new `ui.log-id-preferred-length` option. diff --git a/src/cli_util.rs b/src/cli_util.rs index bd6ac481e..046e075f8 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -1561,7 +1561,6 @@ fn load_template_aliases( // TODO: If/when this logic is relevant in the `lib` crate, make this into // and enum similar to `ColorChoice`. let id_template = match settings.unique_prefixes().as_str() { - "brackets" => format!("id.shortest({desired_id_len}).with_brackets()"), "styled" => format!("id.shortest({desired_id_len})"), _ => format!("id.short({desired_id_len})"), }; diff --git a/src/config-schema.json b/src/config-schema.json index 21274b5fe..9b8dd8f97 100644 --- a/src/config-schema.json +++ b/src/config-schema.json @@ -110,7 +110,6 @@ "unique-prefixes": { "enum": [ "none", - "brackets", "styled" ], "description": "How formatter indicates the unique prefix part of a revision or change ID", diff --git a/src/template_parser.rs b/src/template_parser.rs index bd9f1efd5..403440dba 100644 --- a/src/template_parser.rs +++ b/src/template_parser.rs @@ -910,15 +910,6 @@ fn build_shortest_id_prefix_method<'a, I: 'a>( TemplatePropertyFn(|id: &ShortestIdPrefix| id.rest.clone()), )) } - "with_brackets" => { - // TODO: If we had a map function, this could be expressed as a template - // like 'id.shortest() % (.prefix() if(.rest(), "[" .rest() "]"))' - expect_no_arguments(function)?; - Property::String(chain_properties( - self_property, - TemplatePropertyFn(|id: &ShortestIdPrefix| id.with_brackets()), - )) - } _ => { return Err(TemplateParseError::no_such_method( "ShortestIdPrefix", diff --git a/src/templater.rs b/src/templater.rs index fca7c9a93..f2e9100df 100644 --- a/src/templater.rs +++ b/src/templater.rs @@ -613,16 +613,6 @@ pub struct ShortestIdPrefix { pub rest: String, } -impl ShortestIdPrefix { - pub fn with_brackets(&self) -> String { - if self.rest.is_empty() { - self.prefix.clone() - } else { - format!("{}[{}]", self.prefix, self.rest) - } - } -} - impl Template<()> for ShortestIdPrefix { fn format(&self, _: &(), formatter: &mut dyn Formatter) -> io::Result<()> { formatter.with_label("prefix", |fmt| fmt.write_str(&self.prefix))?; diff --git a/tests/test_commit_template.rs b/tests/test_commit_template.rs index e92d58f2b..a4bb355d4 100644 --- a/tests/test_commit_template.rs +++ b/tests/test_commit_template.rs @@ -81,38 +81,6 @@ fn test_log_default() { (empty) (no description set) "###); - // Test default log output format with bracket prefixes - let stdout = test_env.jj_cmd_success( - &repo_path, - &["log", "--config-toml", "ui.unique-prefixes='brackets'"], - ); - insta::assert_snapshot!(stdout, @r###" - @ k[kmpptxzrspx] test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9[de54178d59d] - │ (empty) description 1 - o q[pvuntsmwlqt] test.user@example.com 2001-02-03 04:05:08.000 +07:00 4[291e264ae97] - │ add a file - o z[zzzzzzzzzzz] 1970-01-01 00:00:00.000 +00:00 0[00000000000] - (empty) (no description set) - "###); - let stdout = test_env.jj_cmd_success( - &repo_path, - &[ - "log", - "--config-toml", - "ui.unique-prefixes='brackets'", - "--config-toml", - "ui.log-id-preferred-length=2", - ], - ); - insta::assert_snapshot!(stdout, @r###" - @ k[k] test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9[d] - │ (empty) description 1 - o q[p] test.user@example.com 2001-02-03 04:05:08.000 +07:00 4[2] - │ add a file - o z[z] 1970-01-01 00:00:00.000 +00:00 0[0] - (empty) (no description set) - "###); - // Test default log output format with styled prefixes and color let stdout = test_env.jj_cmd_success( &repo_path, diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs index db650cb4d..bd998fb54 100644 --- a/tests/test_log_command.rs +++ b/tests/test_log_command.rs @@ -284,7 +284,7 @@ fn test_log_with_or_without_diff() { } #[test] -fn test_log_prefix_highlight_brackets() { +fn test_log_shortest_accessors() { 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"); @@ -301,27 +301,12 @@ fn test_log_prefix_highlight_brackets() { "###, ); - fn prefix_format(len: Option) -> String { - format!( - r#" - "Change " change_id.shortest({0}).with_brackets() " " description.first_line() - " " commit_id.shortest({0}).with_brackets() " " branches - "#, - len.map(|l| l.to_string()).unwrap_or(String::default()) - ) - } - 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(Some(12))]), - @r###" - @ Change q[pvuntsmwlqt] initial b[a1a30916d29] original - │ - ~ - "### - ); + render("original", r#"format_id(change_id) " " format_id(commit_id)"#), + @"q[pvuntsmwlqt] b[a1a30916d29]"); // Create a chain of 10 commits for i in 1..10 { @@ -334,61 +319,24 @@ fn test_log_prefix_highlight_brackets() { } insta::assert_snapshot!( - test_env.jj_cmd_success(&repo_path, &["log", "-r", "original", "-T", &prefix_format(Some(12))]), - @r###" - o Change qpv[untsmwlqt] initial ba1[a30916d29] original - │ - ~ - "### - ); + render("original", r#"format_id(change_id) " " format_id(commit_id)"#), + @"qpv[untsmwlqt] ba1[a30916d29]"); + insta::assert_snapshot!( - test_env.jj_cmd_success(&repo_path, &["log", "-r", ":@", "-T", &prefix_format(Some(12))]), + render(":@", r#"change_id.shortest() " " commit_id.shortest() "\n""#), @r###" - @ Change wq[nwkozpkust] commit9 03[f51310b83e] - o Change km[kuslswpqwq] commit8 f7[7fb1909080] - o Change kp[qxywonksrl] commit7 e7[15ad5db646] - o Change zn[kkpsqqskkl] commit6 38[622e54e2e5] - o Change yo[stqsxwqrlt] commit5 0cf[42f60199c] - o Change vr[uxwmqvtpmx] commit4 9e[6015e4e622] - o Change yq[osqzytrlsw] commit3 06f[34d9b1475] - o Change ro[yxmykxtrkr] commit2 1f[99a5e19891] - o Change mz[vwutvlkqwt] commit1 7b[1f7dee65b4] - o Change qpv[untsmwlqt] initial ba1[a30916d29] original - o Change zzz[zzzzzzzzz] 00[0000000000] - "### - ); - insta::assert_snapshot!( - test_env.jj_cmd_success(&repo_path, &["log", "-r", ":@", "-T", &prefix_format(Some(3))]), - @r###" - @ Change wq[n] commit9 03[f] - o Change km[k] commit8 f7[7] - o Change kp[q] commit7 e7[1] - o Change zn[k] commit6 38[6] - o Change yo[s] commit5 0cf - o Change vr[u] commit4 9e[6] - o Change yq[o] commit3 06f - o Change ro[y] commit2 1f[9] - o Change mz[v] commit1 7b[1] - o Change qpv initial ba1 original - o Change zzz 00[0] - "### - ); - insta::assert_snapshot!( - test_env.jj_cmd_success(&repo_path, &["log", "-r", ":@", "-T", &prefix_format(None)]), - @r###" - @ Change wq commit9 03 - o Change km commit8 f7 - o Change kp commit7 e7 - o Change zn commit6 38 - o Change yo commit5 0cf - o Change vr commit4 9e - o Change yq commit3 06f - o Change ro commit2 1f - o Change mz commit1 7b - o Change qpv initial ba1 original - o Change zzz 00 - "### - ); + wq 03 + km f7 + kp e7 + zn 38 + yo 0cf + vr 9e + yq 06f + ro 1f + mz 7b + qpv ba1 + zzz 00 + "###); insta::assert_snapshot!( render(":@", r#"format_id(change_id) " " format_id(commit_id) "\n""#), @@ -538,10 +486,16 @@ 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"); + test_env.add_config( + r###" + [template-aliases] + 'format_id(id)' = 'id.shortest(12).prefix() "[" id.shortest(12).rest() "]"' + "###, + ); let prefix_format = r#" - "Change " change_id.shortest(12).with_brackets() " " description.first_line() - " " commit_id.shortest(12).with_brackets() " " branches + "Change " format_id(change_id) " " description.first_line() + " " format_id(commit_id) " " branches "#; std::fs::write(repo_path.join("file"), "original file\n").unwrap();