From 00cb12df6c166332c412c4a522db3fe186011d80 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 11 Feb 2024 10:09:31 -0800 Subject: [PATCH] log: optionally render elided parts of the graph as a synthetic node This adds a config to render a synthetic node with a "(elided revisions)" description for elided segments of the graph. I didn't add any templating support for the elided nodes because I'm not sure how we would want that to work. In particular, I don't know what `commit_id` and most other keywords should return for elided revisions. --- CHANGELOG.md | 4 ++ cli/src/commands/log.rs | 58 ++++++++++++++++++--- cli/src/config/colors.toml | 13 ++--- cli/src/graphlog.rs | 16 ++++-- cli/tests/test_log_command.rs | 98 +++++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 003b3ccccf9..889360226da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj git fetch` now accepts `-b` as a shorthand for `--branch`, making it more consistent with other commands that accept a branch +* Set config `log.synthetic-elided-nodes = true` to make `jj log` include + synthetic nodes in the graph where some revisions were elided + ([#1252](https://github.com/martinvonz/jj/issues/1252), [#2971](https://github.com/martinvonz/jj/issues/2971)). This may become the default depending on feedback. + ### Fixed bugs * On Windows, symlinks in the repo are now materialized as regular files in the diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 58bad83a55a..7f394f7087b 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -14,6 +14,7 @@ use itertools::Itertools; use jj_lib::backend::CommitId; +use jj_lib::object_id::ObjectId; use jj_lib::repo::Repo; use jj_lib::revset::{self, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; use jj_lib::revset_graph::{ @@ -104,6 +105,11 @@ pub(crate) fn cmd_log( Some(value) => value.to_string(), None => command.settings().config().get_string("templates.log")?, }; + let use_elided_nodes = command + .settings() + .config() + .get_bool("log.synthetic-elided-nodes") + .unwrap_or_default(); let template = workspace_command.parse_commit_template(&template_string)?; let with_content_format = LogContentFormat::new(ui, command.settings())?; @@ -115,6 +121,7 @@ pub(crate) fn cmd_log( if !args.no_graph { let mut graph = get_graphlog(command.settings(), formatter.raw()); let default_node_symbol = graph.default_node_symbol().to_owned(); + let elided_node_symbol = graph.elided_node_symbol().to_owned(); let forward_iter = TopoGroupedRevsetGraphIterator::new(revset.iter_graph()); let iter: Box> = if args.reversed { Box::new(ReverseRevsetGraphIterator::new(forward_iter)) @@ -127,19 +134,36 @@ pub(crate) fn cmd_log( // the missing edges since we don't care about where they point here // anyway? let mut has_missing = false; + let mut elided = vec![]; for edge in edges { match edge.edge_type { RevsetGraphEdgeType::Missing => { has_missing = true; } - RevsetGraphEdgeType::Direct => graphlog_edges.push(Edge::Present { - direct: true, - target: edge.target, - }), - RevsetGraphEdgeType::Indirect => graphlog_edges.push(Edge::Present { - direct: false, - target: edge.target, - }), + RevsetGraphEdgeType::Direct => { + graphlog_edges.push(Edge::Present { + direct: true, + target: edge.target, + }); + } + RevsetGraphEdgeType::Indirect => { + if use_elided_nodes { + let mut synthetic_commit_id_bytes = edge.target.to_bytes(); + // Let's hope this commit id doesn't exist + synthetic_commit_id_bytes.push(0); + let synthetic_commit_id = CommitId::new(synthetic_commit_id_bytes); + elided.push((synthetic_commit_id.clone(), edge.target)); + graphlog_edges.push(Edge::Present { + direct: true, + target: synthetic_commit_id, + }); + } else { + graphlog_edges.push(Edge::Present { + direct: false, + target: edge.target, + }); + } + } } } if has_missing { @@ -178,6 +202,24 @@ pub(crate) fn cmd_log( node_symbol, &String::from_utf8_lossy(&buffer), )?; + for (synthetic_target, actual_target) in elided { + let edges = [Edge::Present { + direct: true, + target: actual_target, + }]; + let mut buffer = vec![]; + with_content_format.write_graph_text( + ui.new_formatter(&mut buffer).as_mut(), + |formatter| writeln!(formatter.labeled("elided"), "(elided revisions)"), + || graph.width(&synthetic_target, &edges), + )?; + graph.add_node( + &synthetic_target, + &edges, + &elided_node_symbol, + &String::from_utf8_lossy(&buffer), + )?; + } } } else { let iter: Box> = if args.reversed { diff --git a/cli/src/config/colors.toml b/cli/src/config/colors.toml index 6f9f2d6b0af..a9fde235269 100644 --- a/cli/src/config/colors.toml +++ b/cli/src/config/colors.toml @@ -10,10 +10,10 @@ "change_id" = "magenta" # Unique prefixes and the rest for change & commit ids -"prefix" = { bold = true} +"prefix" = { bold = true } "rest" = "bright black" "divergent rest" = "red" -"divergent prefix" = {fg = "red", underline=true} +"divergent prefix" = { fg = "red", underline = true } "hidden prefix" = "default" "email" = "yellow" @@ -28,13 +28,14 @@ "git_refs" = "green" "git_head" = "green" "divergent" = "red" -"divergent change_id"="red" +"divergent change_id" = "red" "conflict" = "red" "empty" = "green" "placeholder" = "red" "description placeholder" = "yellow" "empty description placeholder" = "green" "separator" = "bright black" +"elided" = "bright black" "root" = "green" "working_copy" = { bold = true } @@ -45,13 +46,13 @@ "working_copy email" = "yellow" "working_copy timestamp" = "bright cyan" "working_copy working_copies" = "bright magenta" -"working_copy branch" = "bright magenta" +"working_copy branch" = "bright magenta" "working_copy branches" = "bright magenta" "working_copy local_branches" = "bright magenta" "working_copy remote_branches" = "bright magenta" "working_copy tags" = "bright magenta" "working_copy git_refs" = "bright green" -"working_copy divergent" = "bright red" +"working_copy divergent" = "bright red" "working_copy divergent change_id" = "bright red" "working_copy conflict" = "bright red" "working_copy empty" = "bright green" @@ -71,5 +72,5 @@ "op_log time" = "cyan" "op_log current_operation" = { bold = true } "op_log current_operation id" = "bright blue" -"op_log current_operation user" = "yellow" # No bright yellow, see comment above +"op_log current_operation user" = "yellow" # No bright yellow, see comment above "op_log current_operation time" = "bright cyan" diff --git a/cli/src/graphlog.rs b/cli/src/graphlog.rs index 0278864aa90..b733ceea9c2 100644 --- a/cli/src/graphlog.rs +++ b/cli/src/graphlog.rs @@ -58,6 +58,8 @@ pub trait GraphLog { fn default_node_symbol(&self) -> &str; + fn elided_node_symbol(&self) -> &str; + fn width(&self, id: &K, edges: &[Edge]) -> usize; } @@ -65,6 +67,7 @@ pub struct SaplingGraphLog<'writer, R> { renderer: R, writer: &'writer mut dyn Write, default_node_symbol: String, + elided_node_symbol: String, } impl From<&Edge> for Ancestor { @@ -106,6 +109,10 @@ where &self.default_node_symbol } + fn elided_node_symbol(&self) -> &str { + &self.elided_node_symbol + } + fn width(&self, id: &K, edges: &[Edge]) -> usize { let parents = edges.iter().map_into().collect(); let w: u64 = self.renderer.width(Some(id), Some(&parents)); @@ -118,6 +125,7 @@ impl<'writer, R> SaplingGraphLog<'writer, R> { renderer: R, formatter: &'writer mut dyn Write, default_node_symbol: &str, + elided_node_symbol: &str, ) -> Box + 'writer> where K: Clone + Eq + Hash + 'writer, @@ -127,6 +135,7 @@ impl<'writer, R> SaplingGraphLog<'writer, R> { renderer, writer: formatter, default_node_symbol: default_node_symbol.to_owned(), + elided_node_symbol: elided_node_symbol.to_owned(), }) } } @@ -142,9 +151,10 @@ pub fn get_graphlog<'a, K: Clone + Eq + Hash + 'a>( builder.build_box_drawing().with_square_glyphs(), formatter, "◉", + "◌", ), - "ascii" => SaplingGraphLog::create(builder.build_ascii(), formatter, "o"), - "ascii-large" => SaplingGraphLog::create(builder.build_ascii_large(), formatter, "o"), - "curved" | _ => SaplingGraphLog::create(builder.build_box_drawing(), formatter, "◉"), + "ascii" => SaplingGraphLog::create(builder.build_ascii(), formatter, "o", "."), + "ascii-large" => SaplingGraphLog::create(builder.build_ascii_large(), formatter, "o", "."), + "curved" | _ => SaplingGraphLog::create(builder.build_box_drawing(), formatter, "◉", "◌"), } } diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index 32d1220a2bb..95ce559ac90 100644 --- a/cli/tests/test_log_command.rs +++ b/cli/tests/test_log_command.rs @@ -1333,3 +1333,101 @@ fn test_log_word_wrap() { merge "###); } + +#[test] +fn test_elided() { + // Test that elided commits are shown as synthetic nodes. + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "initial"]); + test_env.jj_cmd_ok(&repo_path, &["new", "-m", "main branch 1"]); + test_env.jj_cmd_ok(&repo_path, &["new", "-m", "main branch 2"]); + test_env.jj_cmd_ok(&repo_path, &["new", "@--", "-m", "side branch 1"]); + test_env.jj_cmd_ok(&repo_path, &["new", "-m", "side branch 2"]); + test_env.jj_cmd_ok( + &repo_path, + &["new", "-m", "merge", r#"description("main branch 2")"#, "@"], + ); + + let get_log = |revs: &str| -> String { + test_env.jj_cmd_success( + &repo_path, + &["log", "-T", r#"description ++ "\n""#, "-r", revs], + ) + }; + + // Test the setup + insta::assert_snapshot!(get_log("::"), @r###" + @ merge + ├─╮ + │ ◉ side branch 2 + │ │ + │ ◉ side branch 1 + │ │ + ◉ │ main branch 2 + │ │ + ◉ │ main branch 1 + ├─╯ + ◉ initial + │ + ◉ + "###); + + // Elide some commits from each side of the merge. It's unclear that a revision + // was skipped on the left side. + insta::assert_snapshot!(get_log("@ | @- | description(initial)"), @r###" + @ merge + ├─╮ + │ ◉ side branch 2 + │ ╷ + ◉ ╷ main branch 2 + ├─╯ + ◉ initial + │ + ~ + "###); + + // Elide shared commits. It's unclear that a revision was skipped on the right + // side (#1252). + insta::assert_snapshot!(get_log("@-- | root()"), @r###" + ◉ side branch 1 + ╷ + ╷ ◉ main branch 1 + ╭─╯ + ◉ + "###); + + // Now test the same thing with synthetic nodes for elided commits + + // Elide some commits from each side of the merge + test_env.add_config("log.synthetic-elided-nodes = true"); + insta::assert_snapshot!(get_log("@ | @- | description(initial)"), @r###" + @ merge + ├─╮ + │ ◉ side branch 2 + │ │ + │ ◌ (elided revisions) + ◉ │ main branch 2 + │ │ + ◌ │ (elided revisions) + ├─╯ + ◉ initial + │ + ~ + "###); + + // Elide shared commits. To keep the implementation simple, it still gets + // rendered as two synthetic nodes. + insta::assert_snapshot!(get_log("@-- | root()"), @r###" + ◉ side branch 1 + │ + ◌ (elided revisions) + │ ◉ main branch 1 + │ │ + │ ◌ (elided revisions) + ├─╯ + ◉ + "###); +}