graphlog: enable Sapling's graph styles by default

I would also rename the feature, but I hope we can instead soon make
it a non-optional dependency and delete the feature.

CHANGELOG.md

@@ -86,6 +86,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * The `[alias]` config section was renamed to `[aliases]`. The old name is
   still accepted for backwards compatibility for some time.
 
+* Commands that draw an ASCII graph (`jj log`, `jj op log`, `jj obslog`) now
+  have different styles available by setting e.g. `ui.graph.format = "curved"`.
+
 ### Fixed bugs
 
 * When sharing the working copy with a Git repo, we used to forget to export

Cargo.toml

@@ -42,7 +42,7 @@ config = { version = "0.13.3", default-features = false, features = ["toml"] }
 crossterm = { version = "0.25", default-features = false }
 dirs = "4.0.0"
 git2 = "0.16.1"
-esl01-renderdag = { version = "0.3.0", optional = true }
+esl01-renderdag = "0.3.0"
 glob = "0.3.1"
 hex = "0.4.3"
 itertools = "0.10.5"
@@ -77,6 +77,5 @@ test-case = "2.2.2"
 testutils = { path = "lib/testutils" }
 
 [features]
-sapling = ["esl01-renderdag"]
 default = ["jujutsu-lib/legacy-thrift"]
 vendored-openssl = ["git2/vendored-openssl", "jujutsu-lib/vendored-openssl"]

docs/config.md

@@ -50,6 +50,14 @@ This setting overrides the `NO_COLOR` environment variable (if set).
 ui.color = "never" # Turn off color
 ```
 
+### Graph style
+
+```toml
+# Possible values: "curved", "square", "ascii", "ascii-large",
+# "legacy" (default)
+ui.graph.format = "curved"
+```
+
 ### Shortest unique prefixes for ids
 
 ```toml

flake.nix

@@ -50,9 +50,6 @@
 
               cargoLock = {
                 lockFile = "${self}/Cargo.lock";
-                outputHashes = {
-                  "renderdag-0.1.0" = "sha256-isOd0QBs5StHpj0xRwSPG40juNvnntyHPW7mT4zsPbM";
-                };
               };
               nativeBuildInputs = [
                 gzip

lib/src/settings.rs

@@ -156,7 +156,7 @@ impl UserSettings {
     pub fn graph_format(&self) -> String {
         self.config
             .get_string("ui.graph.format")
-            .unwrap_or_else(|_| "ascii".to_string())
+            .unwrap_or_else(|_| "legacy".to_string())
     }
 }
 

src/graphlog.rs

@@ -15,8 +15,11 @@
 use std::hash::Hash;
 use std::io;
 use std::io::Write;
+use std::marker::PhantomData;
 
+use itertools::Itertools;
 use jujutsu_lib::settings::UserSettings;
+use renderdag::{Ancestor, GraphRowRenderer, Renderer};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 // An edge to another node in the graph
@@ -55,107 +58,80 @@ pub trait GraphLog<K: Clone + Eq + Hash> {
     ) -> io::Result<()>;
 }
 
-#[cfg(feature = "sapling")]
+pub struct SaplingGraphLog<'writer, K, R: Renderer<K, Output = String>> {
-mod sapling {
+    renderer: R,
-    use std::hash::Hash;
+    writer: &'writer mut dyn Write,
-    use std::io::{self, Write};
+    phantom: PhantomData<K>,
-    use std::marker::PhantomData;
+}
 
-    use itertools::Itertools;
+impl<K: Clone> From<&Edge<K>> for Ancestor<K> {
-    use renderdag::{Ancestor, Renderer};
+    fn from(e: &Edge<K>) -> Self {
-
+        match e {
-    use super::{Edge, GraphLog};
+            Edge::Present {
-
+                target,
-    pub struct SaplingGraphLog<'writer, K, R: Renderer<K, Output = String>> {
+                direct: true,
-        renderer: R,
+            } => Ancestor::Parent(target.clone()),
-        writer: &'writer mut dyn Write,
+            Edge::Present { target, .. } => Ancestor::Ancestor(target.clone()),
-        phantom: PhantomData<K>,
+            Edge::Missing => Ancestor::Anonymous,
-    }
-
-    impl<K: Clone> From<&Edge<K>> for Ancestor<K> {
-        fn from(e: &Edge<K>) -> Self {
-            match e {
-                Edge::Present {
-                    target,
-                    direct: true,
-                } => Ancestor::Parent(target.clone()),
-                Edge::Present { target, .. } => Ancestor::Ancestor(target.clone()),
-                Edge::Missing => Ancestor::Anonymous,
-            }
-        }
-    }
-
-    impl<'writer, K, R> GraphLog<K> for SaplingGraphLog<'writer, K, R>
-    where
-        K: Clone + Eq + Hash,
-        R: Renderer<K, Output = String>,
-    {
-        fn add_node(
-            &mut self,
-            id: &K,
-            edges: &[Edge<K>],
-            node_symbol: &str,
-            text: &str,
-        ) -> io::Result<()> {
-            let row = self.renderer.next_row(
-                id.clone(),
-                edges.iter().map_into().collect(),
-                node_symbol.into(),
-                text.into(),
-            );
-
-            write!(self.writer, "{row}")
-        }
-    }
-
-    impl<'writer, K, R> SaplingGraphLog<'writer, K, R>
-    where
-        K: Clone + Eq + Hash + 'writer,
-        R: Renderer<K, Output = String> + 'writer,
-    {
-        pub fn create(
-            renderer: R,
-            formatter: &'writer mut dyn Write,
-        ) -> Box<dyn GraphLog<K> + 'writer> {
-            Box::new(SaplingGraphLog {
-                renderer,
-                writer: formatter,
-                phantom: PhantomData,
-            })
         }
     }
 }
 
+impl<'writer, K, R> GraphLog<K> for SaplingGraphLog<'writer, K, R>
+where
+    K: Clone + Eq + Hash,
+    R: Renderer<K, Output = String>,
+{
+    fn add_node(
+        &mut self,
+        id: &K,
+        edges: &[Edge<K>],
+        node_symbol: &str,
+        text: &str,
+    ) -> io::Result<()> {
+        let row = self.renderer.next_row(
+            id.clone(),
+            edges.iter().map_into().collect(),
+            node_symbol.into(),
+            text.into(),
+        );
+
+        write!(self.writer, "{row}")
+    }
+}
+
+impl<'writer, K, R> SaplingGraphLog<'writer, K, R>
+where
+    K: Clone + Eq + Hash + 'writer,
+    R: Renderer<K, Output = String> + 'writer,
+{
+    pub fn create(
+        renderer: R,
+        formatter: &'writer mut dyn Write,
+    ) -> Box<dyn GraphLog<K> + 'writer> {
+        Box::new(SaplingGraphLog {
+            renderer,
+            writer: formatter,
+            phantom: PhantomData,
+        })
+    }
+}
+
 pub fn get_graphlog<'a, K: Clone + Eq + Hash + 'a>(
-    #[allow(unused)] settings: &UserSettings,
+    settings: &UserSettings,
     formatter: &'a mut dyn Write,
 ) -> Box<dyn GraphLog<K> + 'a> {
-    #[cfg(feature = "sapling")]
+    let builder = GraphRowRenderer::new().output().with_min_row_height(0);
-    {
-        use renderdag::GraphRowRenderer;
-        use sapling::SaplingGraphLog;
 
-        let builder = GraphRowRenderer::new().output().with_min_row_height(0);
+    match settings.graph_format().as_str() {
-
+        "curved" => SaplingGraphLog::create(builder.build_box_drawing(), formatter),
-        match settings.graph_format().as_str() {
+        "square" => {
-            "curved" => return SaplingGraphLog::create(builder.build_box_drawing(), formatter),
+            SaplingGraphLog::create(builder.build_box_drawing().with_square_glyphs(), formatter)
-            "square" => {
-                return SaplingGraphLog::create(
-                    builder.build_box_drawing().with_square_glyphs(),
-                    formatter,
-                );
-            }
-            "ascii-alternative" => {
-                return SaplingGraphLog::create(builder.build_ascii(), formatter)
-            }
-            "ascii-large" => {
-                return SaplingGraphLog::create(builder.build_ascii_large(), formatter)
-            }
-            _ => {}
         }
-    };
+        "ascii" => SaplingGraphLog::create(builder.build_ascii(), formatter),
-
+        "ascii-large" => SaplingGraphLog::create(builder.build_ascii_large(), formatter),
-    Box::new(AsciiGraphDrawer::new(formatter))
+        _ => Box::new(AsciiGraphDrawer::new(formatter)),
+    }
 }
 
 pub struct AsciiGraphDrawer<'writer, K> {

tests/test_log_command.rs

@@ -681,3 +681,109 @@ fn test_graph_template_color() {
     o (no description set)
     "###);
 }
+
+#[test]
+fn test_graph_styles() {
+    // Test that different graph styles are available.
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    test_env.jj_cmd_success(&repo_path, &["commit", "-m", "initial"]);
+    test_env.jj_cmd_success(&repo_path, &["commit", "-m", "main branch 1"]);
+    test_env.jj_cmd_success(&repo_path, &["describe", "-m", "main branch 2"]);
+    test_env.jj_cmd_success(
+        &repo_path,
+        &["new", "-m", "side branch\nwith\nlong\ndescription"],
+    );
+    test_env.jj_cmd_success(
+        &repo_path,
+        &["new", "-m", "merge", r#"description("main branch 1")"#, "@"],
+    );
+
+    // Default (legacy) style
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T=description"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @   merge
+    |\  
+    o | side branch
+    | | with
+    | | long
+    | | description
+    o | main branch 2
+    |/  
+    o main branch 1
+    o initial
+    o (no description set)
+    "###);
+
+    // ASCII style
+    test_env.add_config(r#"ui.graph.format = "ascii""#);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T=description"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @    merge
+    |\
+    o |  side branch
+    | |  with
+    | |  long
+    | |  description
+    o |  main branch 2
+    |/
+    o  main branch 1
+    o  initial
+    o  (no description set)
+    "###);
+
+    // Large ASCII style
+    test_env.add_config(r#"ui.graph.format = "ascii-large""#);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T=description"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @     merge
+    |\
+    | \
+    o  |  side branch
+    |  |  with
+    |  |  long
+    |  |  description
+    o  |  main branch 2
+    | /
+    |/
+    o  main branch 1
+    o  initial
+    o  (no description set)
+    "###);
+
+    // Curved style
+    test_env.add_config(r#"ui.graph.format = "curved""#);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T=description"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @    merge
+    ├─╮
+    o │  side branch
+    │ │  with
+    │ │  long
+    │ │  description
+    o │  main branch 2
+    ├─╯
+    o  main branch 1
+    o  initial
+    o  (no description set)
+    "###);
+
+    // Square style
+    test_env.add_config(r#"ui.graph.format = "square""#);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T=description"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @    merge
+    ├─┐
+    o │  side branch
+    │ │  with
+    │ │  long
+    │ │  description
+    o │  main branch 2
+    ├─┘
+    o  main branch 1
+    o  initial
+    o  (no description set)
+    "###);
+}