cli: parse graph node settings strictly

CHANGELOG.md

@@ -11,6 +11,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Breaking changes
 
+* Invalid `ui.graph.style` configuration is now an error.
+
 ### Deprecations
 
 * `jj obslog` is now called `jj evolution-log`/`jj evolog`. `jj obslog` remains

cli/src/commands/evolog.rs

@@ -105,7 +105,7 @@ pub(crate) fn cmd_evolog(
         node_template = workspace_command
             .parse_template(
                 &language,
-                &get_node_template(command.settings()),
+                &get_node_template(command.settings())?,
                 CommitTemplateLanguage::wrap_commit_opt,
             )?
             .labeled("node");
@@ -144,7 +144,7 @@ pub(crate) fn cmd_evolog(
         commits.truncate(n);
     }
     if !args.no_graph {
-        let graph_style = GraphStyle::from_settings(command.settings());
+        let graph_style = GraphStyle::from_settings(command.settings())?;
         let mut graph = get_graphlog(graph_style, formatter.raw());
         for commit in commits {
             let mut edges = vec![];

cli/src/commands/log.rs

@@ -145,7 +145,7 @@ pub(crate) fn cmd_log(
         node_template = workspace_command
             .parse_template(
                 &language,
-                &get_node_template(command.settings()),
+                &get_node_template(command.settings())?,
                 CommitTemplateLanguage::wrap_commit_opt,
             )?
             .labeled("node");
@@ -165,7 +165,7 @@ pub(crate) fn cmd_log(
         let limit = args.limit.or(args.deprecated_limit).unwrap_or(usize::MAX);
 
         if !args.no_graph {
-            let graph_style = GraphStyle::from_settings(command.settings());
+            let graph_style = GraphStyle::from_settings(command.settings())?;
             let mut graph = get_graphlog(graph_style, formatter.raw());
             let forward_iter = TopoGroupedGraphIterator::new(revset.iter_graph());
             let iter: Box<dyn Iterator<Item = _>> = if args.reversed {
@@ -297,7 +297,7 @@ pub(crate) fn cmd_log(
     Ok(())
 }
 
-pub fn get_node_template(settings: &UserSettings) -> String {
+pub fn get_node_template(settings: &UserSettings) -> Result<String, config::ConfigError> {
     node_template_for_key(
         settings,
         "templates.log_node",

cli/src/commands/operation/diff.rs

@@ -217,7 +217,7 @@ pub fn show_op_diff(
         writeln!(formatter)?;
         writeln!(formatter, "Changed commits:")?;
         if show_graph {
-            let graph_style = GraphStyle::from_settings(command.settings());
+            let graph_style = GraphStyle::from_settings(command.settings())?;
             let mut graph = get_graphlog(graph_style, formatter.raw());
 
             let graph_iter =

cli/src/commands/operation/log.rs

@@ -99,7 +99,7 @@ pub fn cmd_op_log(
             .parse_template(
                 ui,
                 &language,
-                &get_node_template(command.settings()),
+                &get_node_template(command.settings())?,
                 OperationTemplateLanguage::wrap_operation,
             )?
             .labeled("node");
@@ -117,7 +117,7 @@ pub fn cmd_op_log(
     let limit = args.limit.or(args.deprecated_limit).unwrap_or(usize::MAX);
     let iter = op_walk::walk_ancestors(slice::from_ref(&current_op)).take(limit);
     if !args.no_graph {
-        let graph_style = GraphStyle::from_settings(command.settings());
+        let graph_style = GraphStyle::from_settings(command.settings())?;
         let mut graph = get_graphlog(graph_style, formatter.raw());
         for op in iter {
             let op = op?;
@@ -152,7 +152,7 @@ pub fn cmd_op_log(
     Ok(())
 }
 
-fn get_node_template(settings: &UserSettings) -> String {
+fn get_node_template(settings: &UserSettings) -> Result<String, config::ConfigError> {
     node_template_for_key(
         settings,
         "templates.op_log_node",

cli/src/config/misc.toml

@@ -13,6 +13,7 @@ max-inline-alternation = 3
 allow-filesets = true
 always-allow-large-revsets = false
 diff-instructions = true
+graph.style = "curved"
 paginate = "auto"
 pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } }
 log-word-wrap = false

cli/src/graphlog.rs

@@ -17,6 +17,7 @@ use std::io;
 use std::io::Write;
 
 use itertools::Itertools;
+use jj_lib::settings::ConfigResultExt as _;
 use jj_lib::settings::UserSettings;
 use renderdag::Ancestor;
 use renderdag::GraphRowRenderer;
@@ -112,11 +113,8 @@ pub enum GraphStyle {
 }
 
 impl GraphStyle {
-    pub fn from_settings(settings: &UserSettings) -> Self {
+    pub fn from_settings(settings: &UserSettings) -> Result<Self, config::ConfigError> {
-        settings
+        settings.config().get("ui.graph.style")
-            .config()
-            .get("ui.graph.style")
-            .unwrap_or(GraphStyle::Curved)
     }
 
     pub fn is_ascii(self) -> bool {
@@ -132,12 +130,12 @@ pub fn node_template_for_key(
     key: &str,
     fallback: &str,
     ascii_fallback: &str,
-) -> String {
+) -> Result<String, config::ConfigError> {
-    let symbol = settings.config().get_string(key);
+    let symbol = settings.config().get_string(key).optional()?;
-    if GraphStyle::from_settings(settings).is_ascii() {
+    if GraphStyle::from_settings(settings)?.is_ascii() {
-        symbol.unwrap_or_else(|_| ascii_fallback.to_owned())
+        Ok(symbol.unwrap_or_else(|| ascii_fallback.to_owned()))
     } else {
-        symbol.unwrap_or_else(|_| fallback.to_owned())
+        Ok(symbol.unwrap_or_else(|| fallback.to_owned()))
     }
 }
 

cli/tests/test_log_command.rs

@@ -1280,6 +1280,16 @@ fn test_graph_styles() {
     ○  initial
     ◆
     "###);
+
+    // Invalid style name
+    let stderr = test_env.jj_cmd_failure(
+        &repo_path,
+        &["log", "--config-toml=ui.graph.style='unknown'"],
+    );
+    insta::assert_snapshot!(stderr, @r###"
+    Config error: enum GraphStyle does not have variant constructor unknown
+    For help, see https://martinvonz.github.io/jj/latest/config/.
+    "###);
 }
 
 #[test]