Skip to content

Commit

Permalink
log: optionally render elided parts of the graph as a synthetic node
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@martinvonz
martinvonz committed Feb 12, 2024
1 parent 8d7a44b commit 4ad0ea7
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 22 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 52 additions & 12 deletions cli/src/commands/log.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;

Expand All @@ -115,42 +119,58 @@ 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<dyn Iterator<Item = _>> = if args.reversed {
Box::new(ReverseRevsetGraphIterator::new(forward_iter))
} else {
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');
Expand All @@ -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<dyn Iterator<Item = CommitId>> = if args.reversed {
Expand Down
7 changes: 6 additions & 1 deletion cli/src/config-schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -381,4 +386,4 @@
}
}
}
}
}
13 changes: 7 additions & 6 deletions cli/src/config/colors.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 }
Expand All @@ -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"
Expand All @@ -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"
1 change: 1 addition & 0 deletions cli/src/config/misc.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
16 changes: 13 additions & 3 deletions cli/src/graphlog.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,16 @@ pub trait GraphLog<K: Clone + Eq + Hash> {

fn default_node_symbol(&self) -> &str;

fn elided_node_symbol(&self) -> &str;

fn width(&self, id: &K, edges: &[Edge<K>]) -> usize;
}

pub struct SaplingGraphLog<'writer, R> {
renderer: R,
writer: &'writer mut dyn Write,
default_node_symbol: String,
elided_node_symbol: String,
}

impl<K: Clone> From<&Edge<K>> for Ancestor<K> {
Expand Down Expand Up @@ -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<K>]) -> usize {
let parents = edges.iter().map_into().collect();
let w: u64 = self.renderer.width(Some(id), Some(&parents));
Expand All @@ -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<dyn GraphLog<K> + 'writer>
where
K: Clone + Eq + Hash + 'writer,
Expand All @@ -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(),
})
}
}
Expand All @@ -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, "◉", "◌"),
}
}
98 changes: 98 additions & 0 deletions cli/tests/test_log_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
├─╯
◉
"###);
}

0 comments on commit 4ad0ea7

Please sign in to comment.