From 57916f0c23d7e134efb14b288bc56a3ee8dc7fa8 Mon Sep 17 00:00:00 2001 From: Soham Chowdhury Date: Wed, 20 Dec 2023 12:48:03 +0100 Subject: [PATCH] Make source positions in logs 1-based for consistency --- topiary/src/atom_collection.rs | 43 +++++++++++++++++++--------- topiary/src/tree_sitter.rs | 52 ++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 16 deletions(-) diff --git a/topiary/src/atom_collection.rs b/topiary/src/atom_collection.rs index db96a26e..34b73baf 100644 --- a/topiary/src/atom_collection.rs +++ b/topiary/src/atom_collection.rs @@ -7,7 +7,9 @@ use std::{ use tree_sitter_facade::Node; -use crate::{Atom, FormatterError, FormatterResult, ScopeCondition, ScopeInformation}; +use crate::{ + tree_sitter::NodeExt, Atom, FormatterError, FormatterResult, ScopeCondition, ScopeInformation, +}; /// A struct that holds sets of node IDs that have line breaks before or after them. /// @@ -205,7 +207,10 @@ impl AtomCollection { } if let Some(parent_id) = self.parent_leaf_nodes.get(&node.id()) { if *parent_id != node.id() { - log::warn!("Skipping because the match occurred below a leaf node: {node:?}"); + log::warn!( + "Skipping because the match occurred below a leaf node: {}", + node.display_one_based() + ); return Ok(()); } } @@ -464,14 +469,14 @@ impl AtomCollection { let parent_ids = [parent_ids, &[id]].concat(); log::debug!( - "CST node: {}{:?} - Named: {}", + "CST node: {}{} - Named: {}", " ".repeat(level), - node, + node.display_one_based(), node.is_named() ); if node.end_byte() == node.start_byte() { - log::debug!("Skipping zero-byte node: {node:?}"); + log::debug!("Skipping zero-byte node: {}", node.display_one_based()); } else if node.child_count() == 0 || self.specified_leaf_nodes.contains(&node.id()) // We treat error nodes as leafs when `tolerate_parsing_errors` is set to true. @@ -511,7 +516,10 @@ impl AtomCollection { // TODO: Pre-populate these let target_node = self.first_leaf(node); - log::debug!("Prepending {atom:?} to node {:?}", target_node,); + log::debug!( + "Prepending {atom:?} to node {}", + target_node.display_one_based() + ); self.prepend.entry(target_node.id()).or_default().push(atom); } @@ -528,7 +536,10 @@ impl AtomCollection { let atom = self.wrap(atom, predicates); let target_node = self.last_leaf(node); - log::debug!("Appending {atom:?} to node {:?}", target_node,); + log::debug!( + "Appending {atom:?} to node {}", + target_node.display_one_based() + ); self.append.entry(target_node.id()).or_default().push(atom); } @@ -560,18 +571,18 @@ impl AtomCollection { if self.multi_line_nodes.contains(&parent_id) { log::debug!( - "Expanding softline to hardline in node {:?} with parent {}: {:?}", - node, + "Expanding softline to hardline in node {} with parent {}: {}", + node.display_one_based(), parent_id, - parent + parent.display_one_based() ); Atom::Hardline } else if spaced { log::debug!( - "Expanding softline to space in node {:?} with parent {}: {:?}", - node, + "Expanding softline to space in node {} with parent {}: {}", + node.display_one_based(), parent_id, - parent + parent.display_one_based() ); Atom::Space } else { @@ -1030,7 +1041,11 @@ fn detect_multi_line_nodes(dfs_nodes: &[Node]) -> HashSet { let end_line = node.end_position().row(); if end_line > start_line { - log::debug!("Multi-line node {}: {:?}", node.id(), node,); + log::debug!( + "Multi-line node {}: {}", + node.id(), + node.display_one_based() + ); return Some(node.id()); } diff --git a/topiary/src/tree_sitter.rs b/topiary/src/tree_sitter.rs index d4aa600b..069bce2b 100644 --- a/topiary/src/tree_sitter.rs +++ b/topiary/src/tree_sitter.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::{collections::HashSet, fmt::Display}; use serde::Serialize; use tree_sitter_facade::{ @@ -27,6 +27,12 @@ pub struct Position { pub column: u32, } +impl Display for Position { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + write!(f, "({},{})", self.row, self.column) + } +} + /// Topiary often needs both the tree-sitter `Query` and the original content /// beloging to the file from which the query was parsed. This struct is a simple /// convenience wrapper that combines the `Query` with its original string. @@ -104,6 +110,27 @@ impl From> for SyntaxNode { } } +/// Extension trait for [`Node`] to allow for 1-based display in logs. +/// +/// (Can't be done as a [`Display`] impl on [`Node`] directly, since that would +/// run into orphan issues. An alternative that would work is a [`Display`] impl +/// on a wrapper struct.) +pub trait NodeExt { + /// Produce a textual representation with 1-based row/column indexes. + fn display_one_based(&self) -> String; +} + +impl<'a> NodeExt for Node<'a> { + fn display_one_based(&self) -> String { + format!( + "{{Node {:?} {} - {}}}", + self.kind(), + Position::from(self.start_position()), + Position::from(self.end_position()), + ) + } +} + #[derive(Debug)] // A struct to statically store the public fields of query match results, // to avoid running queries twice. @@ -112,6 +139,27 @@ struct LocalQueryMatch<'a> { captures: Vec>, } +impl<'a> Display for LocalQueryMatch<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!( + f, + "LocalQueryMatch {{ pattern_index: {}, captures: [ ", + self.pattern_index + )?; + for (index, capture) in self.captures.iter().enumerate() { + if index > 0 { + write!(f, ", ")?; + } + // .node() doesn't provide access to the inner [`tree_sitter`] + // object. As a result, we can't get the index out directly, so we + // skip it for now. + write!(f, "{}", capture.node().display_one_based())?; + } + write!(f, " ] }}")?; + Ok(()) + } +} + /// Applies a query to an input content and returns a collection of atoms. /// /// # Errors @@ -171,7 +219,7 @@ pub fn apply_query( // the end, but we don't know if we get a line_comment capture or not. for m in matches { - log::debug!("Processing match: {m:?}"); + log::debug!("Processing match: {m}"); let mut predicates = QueryPredicates::default();