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 10b46c4 commit ada6b5b
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 17 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
58 changes: 50 additions & 8 deletions cli/src/commands/log.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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())?;

Expand All @@ -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<dyn Iterator<Item = _>> = if args.reversed {
Box::new(ReverseRevsetGraphIterator::new(forward_iter))
Expand All @@ -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 {
Expand Down Expand Up @@ -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<dyn Iterator<Item = CommitId>> = if args.reversed {
Expand Down
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"
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,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"),
_ => 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", "."),
_ => 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("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 ada6b5b

Please sign in to comment.