From 4ad0ea7ebd9bc1fbd96e6489b940334e0e4da9f1 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 | 6 +++ cli/src/commands/log.rs | 64 ++++++++++++++++++----- cli/src/config-schema.json | 7 ++- cli/src/config/colors.toml | 13 ++--- cli/src/config/misc.toml | 1 + cli/src/graphlog.rs | 16 ++++-- cli/tests/test_log_command.rs | 98 +++++++++++++++++++++++++++++++++++ 7 files changed, 183 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77fec99188..dbf3441547 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj next/prev` now infer `--edit` when you're already editing a non-head commit (a commit with children). +* Set config `ui.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 58bad83a55..2ca51b6af4 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -104,6 +104,10 @@ 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("ui.log-synthetic-elided-nodes")?; let template = workspace_command.parse_commit_template(&template_string)?; let with_content_format = LogContentFormat::new(ui, command.settings())?; @@ -115,6 +119,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)) @@ -122,35 +127,50 @@ pub(crate) fn cmd_log( Box::new(forward_iter) }; for (commit_id, edges) in iter.take(args.limit.unwrap_or(usize::MAX)) { + // The graph is keyed by (CommitId, is_synthetic) let mut graphlog_edges = vec![]; // TODO: Should we update RevsetGraphIterator to yield this flag instead of all // the missing edges since we don't care about where they point here // anyway? let mut has_missing = false; + let mut elided_targets = 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, false), + }); + } + RevsetGraphEdgeType::Indirect => { + if use_elided_nodes { + elided_targets.push(edge.target.clone()); + graphlog_edges.push(Edge::Present { + direct: true, + target: (edge.target, true), + }); + } else { + graphlog_edges.push(Edge::Present { + direct: false, + target: (edge.target, false), + }); + } + } } } if has_missing { graphlog_edges.push(Edge::Missing); } let mut buffer = vec![]; - let commit = store.get_commit(&commit_id)?; + let key = (commit_id, false); + let commit = store.get_commit(&key.0)?; with_content_format.write_graph_text( ui.new_formatter(&mut buffer).as_mut(), |formatter| template.format(&commit, formatter), - || graph.width(&commit_id, &graphlog_edges), + || graph.width(&key, &graphlog_edges), )?; if !buffer.ends_with(b"\n") { buffer.push(b'\n'); @@ -166,18 +186,38 @@ pub(crate) fn cmd_log( &diff_formats, )?; } - let node_symbol = if Some(&commit_id) == wc_commit_id { + let node_symbol = if Some(&key.0) == wc_commit_id { "@" } else { &default_node_symbol }; graph.add_node( - &commit_id, + &key, &graphlog_edges, node_symbol, &String::from_utf8_lossy(&buffer), )?; + for elided_target in elided_targets { + let elided_key = (elided_target, true); + let real_key = (elided_key.0.clone(), false); + let edges = [Edge::Present { + direct: true, + target: real_key.clone(), + }]; + 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(&elided_key, &edges), + )?; + graph.add_node( + &elided_key, + &edges, + &elided_node_symbol, + &String::from_utf8_lossy(&buffer), + )?; + } } } else { let iter: Box> = if args.reversed { diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 966d61e50c..c3f0285fa6 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -119,6 +119,11 @@ "description": "Whether to wrap log template output", "default": false }, + "log-synthetic-elided-nodes": { + "type": "boolean", + "description": "Whether to render elided parts of the graph as synthetic nodes.", + "default": false + }, "editor": { "type": "string", "description": "Editor to use for commands that involve editing text" @@ -381,4 +386,4 @@ } } } -} +} \ No newline at end of file diff --git a/cli/src/config/colors.toml b/cli/src/config/colors.toml index 6f9f2d6b0a..a9fde23526 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/config/misc.toml b/cli/src/config/misc.toml index d4684ee202..e247778fdc 100644 --- a/cli/src/config/misc.toml +++ b/cli/src/config/misc.toml @@ -8,6 +8,7 @@ tree-level-conflicts = true paginate = "auto" pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } } log-word-wrap = false +log-synthetic-elided-nodes = false [snapshot] max-new-file-size = "1MiB" diff --git a/cli/src/graphlog.rs b/cli/src/graphlog.rs index a7736d71f1..74daa1c298 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,10 +151,11 @@ 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"), + "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, "◉"), + _ => SaplingGraphLog::create(builder.build_box_drawing(), formatter, "◉", "◌"), } } diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index 32d1220a2b..c2f5a14310 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("ui.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) + ├─╯ + ◉ + "###); +}