From 50ff206f12d5fb747b0a5eba07b9efd21c925a59 Mon Sep 17 00:00:00 2001 From: Aleksey Kuznetsov Date: Mon, 11 Mar 2024 19:18:11 +0500 Subject: [PATCH] graphlog: refactor out node symbols from GraphLog Now as default and elided node symbols come from the config, the next logical step is to use them directly bypassing GraphLog. Note that commands like `jj op log` and `jj obslog` do not use the elided node symbol at all. --- cli/src/commands/log.rs | 4 +-- cli/src/commands/obslog.rs | 2 +- cli/src/commands/operation.rs | 2 +- cli/src/graphlog.rs | 51 +++++------------------------------ lib/src/settings.rs | 27 ++++++++++--------- 5 files changed, 24 insertions(+), 62 deletions(-) diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 8bf661fff1..04432230b1 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -118,8 +118,8 @@ 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 default_node_symbol = command.settings().default_node_symbol(); + let elided_node_symbol = command.settings().elided_node_symbol(); let forward_iter = TopoGroupedRevsetGraphIterator::new(revset.iter_graph()); let iter: Box> = if args.reversed { Box::new(ReverseRevsetGraphIterator::new(forward_iter)) diff --git a/cli/src/commands/obslog.rs b/cli/src/commands/obslog.rs index 069ef8508e..a77b439b4e 100644 --- a/cli/src/commands/obslog.rs +++ b/cli/src/commands/obslog.rs @@ -90,7 +90,7 @@ pub(crate) fn cmd_obslog( } if !args.no_graph { let mut graph = get_graphlog(command.settings(), formatter.raw()); - let default_node_symbol = graph.default_node_symbol().to_owned(); + let default_node_symbol = command.settings().default_node_symbol(); for commit in commits { let mut edges = vec![]; for predecessor in &commit.predecessors() { diff --git a/cli/src/commands/operation.rs b/cli/src/commands/operation.rs index 7df773befe..250f297b04 100644 --- a/cli/src/commands/operation.rs +++ b/cli/src/commands/operation.rs @@ -177,7 +177,7 @@ fn cmd_op_log( let iter = op_walk::walk_ancestors(&head_ops).take(args.limit.unwrap_or(usize::MAX)); if !args.no_graph { let mut graph = get_graphlog(command.settings(), formatter.raw()); - let default_node_symbol = graph.default_node_symbol().to_owned(); + let default_node_symbol = command.settings().default_node_symbol(); for op in iter { let op = op?; let mut edges = vec![]; diff --git a/cli/src/graphlog.rs b/cli/src/graphlog.rs index 9f4da5b622..d92c94c4bd 100644 --- a/cli/src/graphlog.rs +++ b/cli/src/graphlog.rs @@ -37,18 +37,12 @@ pub trait GraphLog { text: &str, ) -> io::Result<()>; - fn default_node_symbol(&self) -> &str; - - fn elided_node_symbol(&self) -> &str; - fn width(&self, id: &K, edges: &[Edge]) -> usize; } pub struct SaplingGraphLog<'writer, R> { renderer: R, writer: &'writer mut dyn Write, - default_node_symbol: String, - elided_node_symbol: String, } impl From<&Edge> for Ancestor { @@ -83,14 +77,6 @@ where write!(self.writer, "{row}") } - fn default_node_symbol(&self) -> &str { - &self.default_node_symbol - } - - fn elided_node_symbol(&self) -> &str { - &self.elided_node_symbol - } - fn width(&self, id: &K, edges: &[Edge]) -> usize { let parents = edges.iter().map_into().collect(); let w: u64 = self.renderer.width(Some(id), Some(&parents)); @@ -102,8 +88,6 @@ impl<'writer, R> SaplingGraphLog<'writer, R> { pub fn create( renderer: R, formatter: &'writer mut dyn Write, - default_node_symbol: &str, - elided_node_symbol: &str, ) -> Box + 'writer> where K: Clone + Eq + Hash + 'writer, @@ -112,8 +96,6 @@ impl<'writer, R> SaplingGraphLog<'writer, R> { Box::new(SaplingGraphLog { renderer, writer: formatter, - default_node_symbol: default_node_symbol.to_owned(), - elided_node_symbol: elided_node_symbol.to_owned(), }) } } @@ -123,34 +105,13 @@ pub fn get_graphlog<'a, K: Clone + Eq + Hash + 'a>( formatter: &'a mut dyn Write, ) -> Box + 'a> { let builder = GraphRowRenderer::new().output().with_min_row_height(0); - - let (default_node_symbol, elided_node_symbol) = settings.node_symbols(); - match settings.graph_style().as_str() { - "square" => SaplingGraphLog::create( - builder.build_box_drawing().with_square_glyphs(), - formatter, - &default_node_symbol, - &elided_node_symbol, - ), - "ascii" => SaplingGraphLog::create( - builder.build_ascii(), - formatter, - &default_node_symbol, - &elided_node_symbol, - ), - "ascii-large" => SaplingGraphLog::create( - builder.build_ascii_large(), - formatter, - &default_node_symbol, - &elided_node_symbol, - ), + "square" => { + SaplingGraphLog::create(builder.build_box_drawing().with_square_glyphs(), formatter) + } + "ascii" => SaplingGraphLog::create(builder.build_ascii(), formatter), + "ascii-large" => SaplingGraphLog::create(builder.build_ascii_large(), formatter), // "curved" - _ => SaplingGraphLog::create( - builder.build_box_drawing(), - formatter, - &default_node_symbol, - &elided_node_symbol, - ), + _ => SaplingGraphLog::create(builder.build_box_drawing(), formatter), } } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index af94683535..d35d08412b 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -242,19 +242,12 @@ impl UserSettings { .unwrap_or_else(|_| "curved".to_string()) } - pub fn node_symbols(&self) -> (String, String) { - let default_node_symbol = self.config.get_string("ui.graph.default_node"); - let elided_node_symbol = self.config.get_string("ui.graph.elided_node"); - match self.graph_style().as_str() { - "ascii" | "ascii-large" => ( - default_node_symbol.unwrap_or("o".to_owned()), - elided_node_symbol.unwrap_or(".".to_owned()), - ), - _ => ( - default_node_symbol.unwrap_or("◉".to_owned()), - elided_node_symbol.unwrap_or("◌".to_owned()), - ), - } + pub fn default_node_symbol(&self) -> String { + self.node_symbol_for_key("ui.graph.default_node", "◉", "o") + } + + pub fn elided_node_symbol(&self) -> String { + self.node_symbol_for_key("ui.graph.elided_node", "◌", ".") } pub fn max_new_file_size(&self) -> Result { @@ -279,6 +272,14 @@ impl UserSettings { pub fn sign_settings(&self) -> SignSettings { SignSettings::from_settings(self) } + + fn node_symbol_for_key(&self, key: &str, fallback: &str, ascii_fallback: &str) -> String { + let symbol = self.config.get_string(key); + match self.graph_style().as_str() { + "ascii" | "ascii-large" => symbol.unwrap_or_else(|_| ascii_fallback.to_owned()), + _ => symbol.unwrap_or_else(|_| fallback.to_owned()), + } + } } /// This Rng uses interior mutability to allow generating random values using an