Make node symbols templatable in the graphs

[algmyr]

No-op change styling wise for now. I suspect there will be room for bikeshedding the default style after this.

Hopefully I'm not stepping on @zummenix's toes too much here after the great idea in #3260 to just use a separate template for the elided node. 😅

Some stuff is a bit odd still, like what template to even use for the elided node? Commit template for the root it happens to be associated with? Just make it not a template and let it be a string?

Still need to update docs.

Hopefully just dropping the symbol configs I added recently is fine, especially considering there hasn't been a release with them included.

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added tests to cover my changes

[zummenix]

Hopefully I'm not stepping on @zummenix's toes too much here after the great idea in #3260 to just use a separate template for the elided node. 😅

Not at all. I appreciate you working on this, thank you!

I skimmed through the changes. Nice work! Seems fine to me, but I'll let owners properly review them.

Some stuff is a bit odd still, like what template to even use for the elided node? Commit template for the root it happens to be associated with? Just make it not a template and let it be a string?

I wondered almost the same questions. I was also wondering should there be different templates for different kinds of graphs (log, op log, obslog)? And users would be able to override them in their config, like for example:

[templates]
graph-log = 'my_fancy_graph_log'
graph-op-log = 'my_simple_graph_log'
graph-obslog = 'my_simple_graph_log'

[algmyr]

Another open question. It seems feasible to include the elided revision text in the template and stop having it be hard-coded, and it seems some people have been eager to have the ability to not have that. Is it worth making the default something like this?

'"◌  " ++ label("elided", "(elided revisions)")'

[martinvonz]

'"◌ " ++ label("elided", "(elided revisions)")'

Then we would have to split that up after reading it from the config (or maybe after rendering it), right? Because the node is passed separately from the text to the graph renderer.

[algmyr]

Then we would have to split that up after reading it from the config (or maybe after rendering it), right? Because the node is passed separately from the text to the graph renderer.

One option is to just write a newline in the graph, but it feels a bit hacky and maybe the two spaces for alignment in the template is not always the right spacing?

         let elided_key = (elided_target, true);
         let real_key = (elided_key.0.clone(), false);
         let edges = [Edge::Direct(real_key)];
-        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),
-        )?;
         let node_symbol = format_template(ui, &(), elided_node_template.as_ref());
         graph.add_node(
             &elided_key,
             &edges,
             &node_symbol,
-            &String::from_utf8_lossy(&buffer),
+            &"\n",
         )?;
     }
 }

[martinvonz]

is not always the right spacing?

Right, there can be lines to the right of the elided node, like this:

◌ │  (elided revisions)
├─╯

[algmyr]

Gotcha, let's just leave things as is then. I guess there could be yet another template introduced in the future if someone really wants it.

[martinvonz]

Looks good to me, thanks, but I'll let @yuja give the final approval since he's already done a few rounds of reviews

[yuja]

Yes, looks good, thanks!

Two questions:

• Will you merge this soon? or rebase atop templater: don't define TemplateLanguage::Context statically #3332?
• Do you like the idea of rendering elided node as None? commit_templater: encode elided node as Option<Commit> #3319 (comment)

(To be clear, I don't intend to block this PR. I can reorder my PRs even if we agree with Option<Commit> idea.)