Skip to content

Commit

Permalink
cli: Color more change and commit ids in cli outputs
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Sep 15, 2024
1 parent 7ff12ec commit 6ab5ab8
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 62 deletions.
44 changes: 41 additions & 3 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
Expand Down
12 changes: 10 additions & 2 deletions cli/src/commands/duplicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)?;
}
Expand Down
73 changes: 35 additions & 38 deletions cli/src/commands/git/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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| {
Expand All @@ -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(())
}
Expand Down
15 changes: 13 additions & 2 deletions cli/src/commands/operation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -158,6 +160,7 @@ pub fn show_op_diff(
current_repo: &dyn Repo,
from_repo: &Arc<ReadonlyRepo>,
to_repo: &Arc<ReadonlyRepo>,
id_prefix_context: &IdPrefixContext,
commit_summary_template: &TemplateRenderer<Commit>,
graph_style: Option<GraphStyle>,
with_content_format: &LogContentFormat,
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -379,10 +386,14 @@ pub fn show_op_diff(
fn write_modified_change_summary(
formatter: &mut dyn Formatter,
commit_summary_template: &TemplateRenderer<Commit>,
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, " ")?;
Expand Down
1 change: 1 addition & 0 deletions cli/src/commands/operation/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions cli/src/commands/operation/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions cli/tests/test_git_private_commits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
"#);
}

Expand All @@ -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)
Expand Down Expand Up @@ -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*')""#);
Expand Down
Loading

0 comments on commit 6ab5ab8

Please sign in to comment.