From 36ab165b571eae5f683e6b5253138755e630852d Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 3 Sep 2024 17:01:22 +0900 Subject: [PATCH] cli: parse graph node settings strictly --- CHANGELOG.md | 2 ++ cli/src/commands/evolog.rs | 4 ++-- cli/src/commands/log.rs | 6 +++--- cli/src/commands/operation/diff.rs | 2 +- cli/src/commands/operation/log.rs | 6 +++--- cli/src/config/misc.toml | 1 + cli/src/graphlog.rs | 18 ++++++++---------- cli/tests/test_log_command.rs | 10 ++++++++++ 8 files changed, 30 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea31813fc..1b01eb3ba 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cli/src/commands/evolog.rs b/cli/src/commands/evolog.rs index 290945896..4a79e57c6 100644 --- a/cli/src/commands/evolog.rs +++ b/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![]; diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 7969537bc..b72cd41a7 100644 --- a/cli/src/commands/log.rs +++ b/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> = 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 { node_template_for_key( settings, "templates.log_node", diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs index 6985aa9a1..a96d8d903 100644 --- a/cli/src/commands/operation/diff.rs +++ b/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 = diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index 8fe01255a..00b96e8ae 100644 --- a/cli/src/commands/operation/log.rs +++ b/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(¤t_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 { node_template_for_key( settings, "templates.op_log_node", diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml index f7f62232d..db575385e 100644 --- a/cli/src/config/misc.toml +++ b/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 diff --git a/cli/src/graphlog.rs b/cli/src/graphlog.rs index 95f8e257d..4d5f0ab1f 100644 --- a/cli/src/graphlog.rs +++ b/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 { - settings - .config() - .get("ui.graph.style") - .unwrap_or(GraphStyle::Curved) + pub fn from_settings(settings: &UserSettings) -> Result { + settings.config().get("ui.graph.style") } pub fn is_ascii(self) -> bool { @@ -132,12 +130,12 @@ pub fn node_template_for_key( key: &str, fallback: &str, ascii_fallback: &str, -) -> String { - let symbol = settings.config().get_string(key); - if GraphStyle::from_settings(settings).is_ascii() { - symbol.unwrap_or_else(|_| ascii_fallback.to_owned()) +) -> Result { + let symbol = settings.config().get_string(key).optional()?; + if GraphStyle::from_settings(settings)?.is_ascii() { + 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())) } } diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index 5fe37e27a..49adfdd14 100644 --- a/cli/tests/test_log_command.rs +++ b/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]