From 6ab5ab8869161fa96ee7023f90f9364c1144e72f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 15 Sep 2024 15:04:50 +0200 Subject: [PATCH] cli: Color more change and commit ids in cli outputs --- cli/src/cli_util.rs | 44 ++++++++++++++-- cli/src/commands/duplicate.rs | 12 ++++- cli/src/commands/git/push.rs | 73 +++++++++++++-------------- cli/src/commands/operation/diff.rs | 15 +++++- cli/src/commands/operation/log.rs | 1 + cli/src/commands/operation/show.rs | 1 + cli/tests/test_git_private_commits.rs | 12 ++--- cli/tests/test_git_push.rs | 18 +++---- cli/tests/test_operations.rs | 4 +- 9 files changed, 118 insertions(+), 62 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 55df227a54..b27038bf42 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -14,6 +14,7 @@ use std::borrow::Cow; use std::cell::OnceCell; +use std::cmp::max; use std::collections::BTreeMap; use std::collections::HashSet; use std::env; @@ -1945,9 +1946,7 @@ impl WorkspaceCommandTransaction<'_> { /// Creates commit template language environment capturing the current /// transaction state. pub fn commit_template_language(&self) -> CommitTemplateLanguage<'_> { - let id_prefix_context = self - .id_prefix_context - .get_or_init(|| self.helper.env.new_id_prefix_context()); + let id_prefix_context = self.id_prefix_context(); self.helper .env .commit_template_language(self.tx.repo(), id_prefix_context) @@ -1994,6 +1993,11 @@ impl WorkspaceCommandTransaction<'_> { ); } } + + pub fn id_prefix_context(&self) -> &IdPrefixContext { + self.id_prefix_context + .get_or_init(|| self.helper.env.new_id_prefix_context()) + } } fn find_workspace_dir(cwd: &Path) -> &Path { @@ -2514,6 +2518,40 @@ pub fn edit_temp_file( Ok(edited) } +pub fn format_commit_shortest_prefix( + formatter: &mut dyn Formatter, + repo: &dyn Repo, + id_prefix_context: &IdPrefixContext, + commit_id: &CommitId, +) -> io::Result<()> { + let mut hex = commit_id.hex(); + let prefix_len = id_prefix_context.shortest_commit_prefix_len(repo, commit_id); + hex.truncate(max(prefix_len, 12)); + let rest = hex.split_off(prefix_len); + formatter.with_label("commit_id", |formatter| { + write!(formatter.labeled("prefix"), "{hex}")?; + write!(formatter.labeled("rest"), "{rest}")?; + Ok(()) + }) +} + +pub fn format_change_shortest_prefix( + formatter: &mut dyn Formatter, + repo: &dyn Repo, + id_prefix_context: &IdPrefixContext, + change_id: &ChangeId, +) -> io::Result<()> { + let mut hex = to_reverse_hex(&change_id.hex()).unwrap(); + let prefix_len = id_prefix_context.shortest_change_prefix_len(repo, change_id); + hex.truncate(max(prefix_len, 12)); + let rest = hex.split_off(prefix_len); + formatter.with_label("change_id", |formatter| { + write!(formatter.labeled("prefix"), "{hex}")?; + write!(formatter.labeled("rest"), "{rest}")?; + Ok(()) + }) +} + pub fn short_commit_hash(commit_id: &CommitId) -> String { commit_id.hex()[0..12].to_string() } diff --git a/cli/src/commands/duplicate.rs b/cli/src/commands/duplicate.rs index 667348f644..bbbdc82e53 100644 --- a/cli/src/commands/duplicate.rs +++ b/cli/src/commands/duplicate.rs @@ -20,7 +20,7 @@ use jj_lib::commit::Commit; use jj_lib::repo::Repo; use tracing::instrument; -use crate::cli_util::short_commit_hash; +use crate::cli_util::format_commit_shortest_prefix; use crate::cli_util::CommandHelper; use crate::cli_util::RevisionArg; use crate::command_error::user_error; @@ -81,8 +81,16 @@ pub(crate) fn cmd_duplicate( } if let Some(mut formatter) = ui.status_formatter() { + let id_prefix_context = tx.id_prefix_context(); for (old_id, new_commit) in &duplicated_old_to_new { - write!(formatter, "Duplicated {} as ", short_commit_hash(old_id))?; + write!(formatter, "Duplicated ")?; + format_commit_shortest_prefix( + formatter.as_mut(), + &*base_repo, + id_prefix_context, + old_id, + )?; + write!(formatter, " as ")?; tx.write_commit_summary(formatter.as_mut(), new_commit)?; writeln!(formatter)?; } diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index b1523cb82a..d3474c32f8 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -23,6 +23,7 @@ use jj_lib::backend::CommitId; use jj_lib::git; use jj_lib::git::GitBranchPushTargets; use jj_lib::git::GitPushError; +use jj_lib::id_prefix::IdPrefixContext; use jj_lib::object_id::ObjectId; use jj_lib::op_store::RefTarget; use jj_lib::refs::classify_bookmark_push_action; @@ -36,6 +37,7 @@ use jj_lib::settings::UserSettings; use jj_lib::str_util::StringPattern; use jj_lib::view::View; +use crate::cli_util::format_commit_shortest_prefix; use crate::cli_util::short_change_hash; use crate::cli_util::short_commit_hash; use crate::cli_util::CommandHelper; @@ -266,8 +268,15 @@ pub fn cmd_git_push( validate_commits_ready_to_push(&bookmark_updates, &remote, &tx, command, args)?; if let Some(mut formatter) = ui.status_formatter() { - writeln!(formatter, "Changes to push to {remote}:")?; - print_commits_ready_to_push(formatter.as_mut(), repo.as_ref(), &bookmark_updates)?; + write!(formatter, "Changes to push to ")?; + write!(formatter.labeled("remote_branches"), "{remote}")?; + writeln!(formatter, ":")?; + print_commits_ready_to_push( + formatter.as_mut(), + repo.as_ref(), + tx.id_prefix_context(), + &bookmark_updates, + )?; } if args.dry_run { @@ -381,6 +390,7 @@ fn validate_commits_ready_to_push( fn print_commits_ready_to_push( formatter: &mut dyn Formatter, repo: &dyn Repo, + id_prefix_context: &IdPrefixContext, bookmark_updates: &[(String, BookmarkPushUpdate)], ) -> io::Result<()> { let to_direction = |old_target: &CommitId, new_target: &CommitId| { @@ -396,46 +406,33 @@ fn print_commits_ready_to_push( for (bookmark_name, update) in bookmark_updates { match (&update.old_target, &update.new_target) { - (Some(old_target), Some(new_target)) => { - let old = short_commit_hash(old_target); - let new = short_commit_hash(new_target); - // TODO(ilyagr): Add color. Once there is color, "Move bookmark ... sideways" - // may read more naturally than "Move sideways bookmark ...". - // Without color, it's hard to see at a glance if one bookmark - // among many was moved sideways (say). TODO: People on Discord - // suggest "Move bookmark ... forward by n commits", - // possibly "Move bookmark ... sideways (X forward, Y back)". - let msg = match to_direction(old_target, new_target) { - BookmarkMoveDirection::Forward => { - format!("Move forward bookmark {bookmark_name} from {old} to {new}") - } - BookmarkMoveDirection::Backward => { - format!("Move backward bookmark {bookmark_name} from {old} to {new}") - } - BookmarkMoveDirection::Sideways => { - format!("Move sideways bookmark {bookmark_name} from {old} to {new}") - } - }; - writeln!(formatter, " {msg}")?; - } - (Some(old_target), None) => { - writeln!( - formatter, - " Delete bookmark {bookmark_name} from {}", - short_commit_hash(old_target) - )?; - } - (None, Some(new_target)) => { - writeln!( - formatter, - " Add bookmark {bookmark_name} to {}", - short_commit_hash(new_target) - )?; - } + // TODO: People on Discord + // suggest "Move bookmark ... forward by n commits", + // possibly "Move bookmark ... sideways (X forward, Y back)". + (Some(_), Some(_)) => write!(formatter, " Move bookmark ",)?, + (Some(_), None) => write!(formatter, " Delete bookmark ",)?, + (None, Some(_)) => write!(formatter, " Add bookmark ",)?, (None, None) => { panic!("Not pushing any change to bookmark {bookmark_name}"); } } + write!(formatter.labeled("bookmarks"), "{bookmark_name}")?; + if let (Some(old_target), Some(new_target)) = (&update.old_target, &update.new_target) { + match to_direction(old_target, new_target) { + BookmarkMoveDirection::Forward => write!(formatter, " forward")?, + BookmarkMoveDirection::Backward => write!(formatter, " backward")?, + BookmarkMoveDirection::Sideways => write!(formatter, " sideways")?, + } + } + if let Some(old_target) = &update.old_target { + write!(formatter, " from ")?; + format_commit_shortest_prefix(formatter, repo, id_prefix_context, old_target)?; + } + if let Some(new_target) = &update.new_target { + write!(formatter, " to ")?; + format_commit_shortest_prefix(formatter, repo, id_prefix_context, new_target)?; + } + writeln!(formatter)?; } Ok(()) } diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs index 813f028c64..bf0bb8343f 100644 --- a/cli/src/commands/operation/diff.rs +++ b/cli/src/commands/operation/diff.rs @@ -24,6 +24,7 @@ use jj_lib::dag_walk; use jj_lib::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO; use jj_lib::graph::GraphEdge; use jj_lib::graph::TopoGroupedGraphIterator; +use jj_lib::id_prefix::IdPrefixContext; use jj_lib::matchers::EverythingMatcher; use jj_lib::op_store::RefTarget; use jj_lib::op_store::RemoteRef; @@ -35,7 +36,7 @@ use jj_lib::repo::Repo; use jj_lib::revset; use jj_lib::revset::RevsetIteratorExt as _; -use crate::cli_util::short_change_hash; +use crate::cli_util::format_change_shortest_prefix; use crate::cli_util::CommandHelper; use crate::cli_util::LogContentFormat; use crate::command_error::CommandError; @@ -140,6 +141,7 @@ pub fn cmd_op_diff( merged_repo, &from_repo, &to_repo, + &id_prefix_context, &commit_summary_template, (!args.no_graph).then_some(graph_style), &with_content_format, @@ -158,6 +160,7 @@ pub fn show_op_diff( current_repo: &dyn Repo, from_repo: &Arc, to_repo: &Arc, + id_prefix_context: &IdPrefixContext, commit_summary_template: &TemplateRenderer, graph_style: Option, with_content_format: &LogContentFormat, @@ -224,6 +227,8 @@ pub fn show_op_diff( write_modified_change_summary( formatter, commit_summary_template, + current_repo, + id_prefix_context, &change_id, modified_change, ) @@ -258,6 +263,8 @@ pub fn show_op_diff( write_modified_change_summary( formatter, commit_summary_template, + current_repo, + id_prefix_context, &change_id, modified_change, ) @@ -379,10 +386,14 @@ pub fn show_op_diff( fn write_modified_change_summary( formatter: &mut dyn Formatter, commit_summary_template: &TemplateRenderer, + repo: &dyn Repo, + id_prefix_context: &IdPrefixContext, change_id: &ChangeId, modified_change: &ModifiedChange, ) -> Result<(), std::io::Error> { - writeln!(formatter, "Change {}", short_change_hash(change_id))?; + write!(formatter, "Change ")?; + format_change_shortest_prefix(formatter, repo, id_prefix_context, change_id)?; + writeln!(formatter)?; for commit in modified_change.added_commits.iter() { formatter.with_label("diff", |formatter| write!(formatter.labeled("added"), "+"))?; write!(formatter, " ")?; diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index aabba7087d..d0d3d4eaef 100644 --- a/cli/src/commands/operation/log.rs +++ b/cli/src/commands/operation/log.rs @@ -167,6 +167,7 @@ fn do_op_log( repo.as_ref(), &parent_repo, &repo, + &id_prefix_context, &commit_summary_template, (!args.no_graph).then_some(graph_style), with_content_format, diff --git a/cli/src/commands/operation/show.rs b/cli/src/commands/operation/show.rs index 9e228a8bed..2d7795784c 100644 --- a/cli/src/commands/operation/show.rs +++ b/cli/src/commands/operation/show.rs @@ -97,6 +97,7 @@ pub fn cmd_op_show( repo.as_ref(), &parent_repo, &repo, + &id_prefix_context, &commit_summary_template, (!args.no_graph).then_some(graph_style), &with_content_format, diff --git a/cli/tests/test_git_private_commits.rs b/cli/tests/test_git_private_commits.rs index 7946f817c2..4ee2afed1c 100644 --- a/cli/tests/test_git_private_commits.rs +++ b/cli/tests/test_git_private_commits.rs @@ -90,7 +90,7 @@ fn test_git_private_commits_block_pushing() { let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark main from 7eb97bf230ad to aa3058ff8663 + Move bookmark main forward from 7eb97bf230ad to aa3058ff8663 Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it. Working copy now at: znkkpsqq 2e1adf47 (empty) (no description set) Parent commit : yqosqzyt aa3058ff main | (empty) private 1 @@ -118,7 +118,7 @@ fn test_git_private_commits_can_be_overridden() { ); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark main from 7eb97bf230ad to aa3058ff8663 + Move bookmark main forward from 7eb97bf230ad to aa3058ff8663 Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it. Working copy now at: znkkpsqq 2e1adf47 (empty) (no description set) Parent commit : yqosqzyt aa3058ff main | (empty) private 1 @@ -137,7 +137,7 @@ fn test_git_private_commits_are_not_checked_if_immutable() { let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark main from 7eb97bf230ad to aa3058ff8663 + Move bookmark main forward from 7eb97bf230ad to aa3058ff8663 Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it. Working copy now at: yostqsxw dce4a15c (empty) (no description set) Parent commit : yqosqzyt aa3058ff main | (empty) private 1 @@ -173,7 +173,7 @@ fn test_git_private_commits_descending_from_commits_pushed_do_not_block_pushing( let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=main"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark main from 7eb97bf230ad to 05ef53bc99ec + Move bookmark main forward from 7eb97bf230ad to 05ef53bc99ec "#); } @@ -196,7 +196,7 @@ fn test_git_private_commits_already_on_the_remote_do_not_block_push() { test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=main", "-b=bookmark1"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark main from 7eb97bf230ad to fbb352762352 + Move bookmark main forward from 7eb97bf230ad to fbb352762352 Add bookmark bookmark1 to 7eb97bf230ad Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it. Working copy now at: kpqxywon a7b08364 (empty) (no description set) @@ -244,7 +244,7 @@ fn test_git_private_commits_are_evaluated_separately_for_each_remote() { let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=main"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark main from 7eb97bf230ad to d8632ce893ab + Move bookmark main forward from 7eb97bf230ad to d8632ce893ab "#); test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#); diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 4a34c22358..c1309a66f2 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -92,7 +92,7 @@ fn test_git_push_current_bookmark() { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark bookmark2 from 8476341eb395 to bc7610b65a91 + Move bookmark bookmark2 forward from 8476341eb395 to bc7610b65a91 Add bookmark my-bookmark to bc7610b65a91 Dry-run requested, not pushing. "#); @@ -155,7 +155,7 @@ fn test_git_push_parent_bookmark() { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move sideways bookmark bookmark1 from d13ecdbda2a2 to e612d524a5c6 + Move bookmark bookmark1 sideways from d13ecdbda2a2 to e612d524a5c6 "#); } @@ -215,7 +215,7 @@ fn test_git_push_other_remote_has_bookmark() { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move sideways bookmark bookmark1 from d13ecdbda2a2 to a657f1b61b94 + Move bookmark bookmark1 sideways from d13ecdbda2a2 to a657f1b61b94 "#); // Since it's already pushed to origin, nothing will happen if push again let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); @@ -260,7 +260,7 @@ fn test_git_push_forward_unexpectedly_moved() { let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move forward bookmark bookmark1 from d13ecdbda2a2 to 6750425ff51c + Move bookmark bookmark1 forward from d13ecdbda2a2 to 6750425ff51c Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); @@ -300,7 +300,7 @@ fn test_git_push_sideways_unexpectedly_moved() { let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move sideways bookmark bookmark1 from d13ecdbda2a2 to 0f8bf988588e + Move bookmark bookmark1 sideways from d13ecdbda2a2 to 0f8bf988588e Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); @@ -377,7 +377,7 @@ fn test_git_push_unexpectedly_deleted() { let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: - Move sideways bookmark bookmark1 from d13ecdbda2a2 to 1ebe27ba04bf + Move bookmark bookmark1 sideways from d13ecdbda2a2 to 1ebe27ba04bf Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); @@ -483,7 +483,7 @@ fn test_git_push_multiple() { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 - Move sideways bookmark bookmark2 from 8476341eb395 to c4a3c3105d92 + Move bookmark bookmark2 sideways from 8476341eb395 to c4a3c3105d92 Add bookmark my-bookmark to c4a3c3105d92 Dry-run requested, not pushing. "#); @@ -1141,7 +1141,7 @@ fn test_git_push_conflicting_bookmarks() { Warning: Bookmark bookmark2 is conflicted Hint: Run `jj bookmark list` to inspect, and use `jj bookmark set` to fix it up. Changes to push to origin: - Move forward bookmark bookmark1 from d13ecdbda2a2 to 8df52121b022 + Move bookmark bookmark1 forward from d13ecdbda2a2 to 8df52121b022 "#); // --revisions shouldn't be blocked by conflicting bookmark @@ -1152,7 +1152,7 @@ fn test_git_push_conflicting_bookmarks() { Warning: Bookmark bookmark2 is conflicted Hint: Run `jj bookmark list` to inspect, and use `jj bookmark set` to fix it up. Changes to push to origin: - Move forward bookmark bookmark1 from 8df52121b022 to 345e1f64a64d + Move bookmark bookmark1 forward from 8df52121b022 to 345e1f64a64d "#); } diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index 27649eab3b..84c1504bd2 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -1236,7 +1236,7 @@ fn test_op_diff() { "###); insta::assert_snapshot!(&stderr, @r#" Changes to push to origin: - Move forward bookmark bookmark-1 from 4f856199edbf to 358b82d6be53 + Move bookmark bookmark-1 forward from 4f856199edbf to 358b82d6be53 Delete bookmark bookmark-2 from d487febd08e6 Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it. Working copy now at: oupztwtk 2f0718a0 (empty) (no description set) @@ -1932,7 +1932,7 @@ fn test_op_show() { "###); insta::assert_snapshot!(&stderr, @r#" Changes to push to origin: - Move forward bookmark bookmark-1 from 4f856199edbf to eb6c2b21ec20 + Move bookmark bookmark-1 forward from 4f856199edbf to eb6c2b21ec20 Delete bookmark bookmark-2 from d487febd08e6 Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it. Working copy now at: pzsxstzt 7ab2d837 (empty) (no description set)