From d67a363dc4dc955e3ea22595fda22173034584eb Mon Sep 17 00:00:00 2001 From: Erin van der Veen Date: Wed, 8 May 2024 10:37:52 +0200 Subject: [PATCH] fix: use slice patterns over iterator --- topiary-core/src/atom_collection.rs | 185 +++++++++++++++++----------- topiary-core/src/lib.rs | 14 +++ 2 files changed, 130 insertions(+), 69 deletions(-) diff --git a/topiary-core/src/atom_collection.rs b/topiary-core/src/atom_collection.rs index 8845a37c..aa39d50b 100644 --- a/topiary-core/src/atom_collection.rs +++ b/topiary-core/src/atom_collection.rs @@ -784,69 +784,84 @@ impl AtomCollection { log::debug!("List of atoms after post-processing: {:?}", self.atoms); } + /// This function post-processes the atoms in the collection. + /// It modifies the collection in-place, removing unnecessary atoms and adjusting the position of others. fn post_process_inner(&mut self) { - let mut prev: Option<&mut Atom> = None; - for next in &mut self.atoms { - if let Some(prev) = prev.as_mut() { - match prev { - // Discard all spaces following an antispace. We'll fix the - // preceding ones in the next pass. - Atom::Antispace => { - match next { - // Remove any space or antispace that follows an - // antispace by setting it empty. - Atom::Space | Atom::Antispace => { - *next = Atom::Empty; - } - _ => {} - } - } + // A mutable reference to the atoms in the collection. + let mut remaining = &mut self.atoms[..]; + + // The previous atom in the collection. Initialized to the first atom, if it exists. + // We a mutable reference to the previous atom so that we can modify it in-place. + // This atoms holds the last atom encountered that is not empty. + let mut prev: &mut Atom = if let [head, tail @ ..] = remaining { + remaining = tail; + head + } else { + return; + }; - // If the last atom is a space/line - Atom::Empty | Atom::Space | Atom::Hardline | Atom::Blankline => { - match next { - // And the next one is also a space/line - Atom::Empty | Atom::Space | Atom::Hardline | Atom::Blankline => { - // Set the non-dominant one to empty. - if is_dominant(next, prev) { - **prev = Atom::Empty; - } else { - *next = Atom::Empty; - } - } - - // Or an indentation delimiter, then one has to merge/re-order. - Atom::IndentStart | Atom::IndentEnd => { - let old_prev = prev.clone(); - **prev = next.clone(); - *next = old_prev; - } - - _ => {} - } - } + // Set all leading whitespace atoms to empty. + while let Atom::Space | Atom::Antispace | Atom::Hardline | Atom::Blankline = *prev { + *prev = Atom::Empty; + if let [head, tail @ ..] = remaining { + prev = head; + remaining = tail; + } else { + return; + } + } - _ => {} + // Process the remaining atoms in the collection. + while !remaining.is_empty() { + match (prev, remaining) { + // If an antispace atom is followed by a space or another antispace, remove the following atom. + ( + moved_prev @ Atom::Antispace, + [head @ (Atom::Space | Atom::Antispace), tail @ ..], + ) => { + *head = Atom::Empty; + + prev = moved_prev; + remaining = tail; } - } else { - // If we're at the beginning of the file and still haven't - // reached a non-empty atom, we remove all the spaces and - // newlines by setting them empty. - match next { - Atom::Empty - | Atom::Space - | Atom::Antispace - | Atom::Hardline - | Atom::Blankline => { - *next = Atom::Empty; + // If two whitespace atoms follow each other, remove the non-dominant one. + ( + moved_prev @ (Atom::Space | Atom::Hardline | Atom::Blankline), + [head @ (Atom::Space | Atom::Hardline | Atom::Blankline), tail @ ..], + ) => { + if head.dominates(moved_prev) { + *moved_prev = Atom::Empty; + } else { + *head = Atom::Empty; } - _ => {} - }; - } - if *next != Atom::Empty { - // Let prev point to the previous non-empty atom. - prev = Some(next); + prev = moved_prev; + remaining = tail; + } + // If a whitespace atom is followed by an indent atom, swap their positions. + ( + moved_prev @ (Atom::Space | Atom::Hardline | Atom::Blankline), + moved_remaining @ [Atom::IndentStart | Atom::IndentEnd, ..], + ) => { + let old_prev = moved_prev.clone(); + let indent = moved_remaining.first_mut().unwrap(); + *moved_prev = indent.clone(); + *indent = old_prev; + + prev = moved_prev; + remaining = moved_remaining; + } + // If the current atom is not empty, update the previous atom. + (moved_prev, [head, tail @ ..]) => { + prev = if matches!(head, Atom::Empty) { + moved_prev + } else { + head + }; + remaining = tail; + } + // This case should never be reached because we check that `remaining` is not empty at the start of the loop. + (_, []) => unreachable!("remaining cannot be empty"), } } } @@ -970,18 +985,6 @@ fn collapse_spaces_before_antispace(v: &mut [Atom]) { } } -// This function is only expected to take spaces and newlines as argument. -// It defines the order Blankline > Hardline > Space > Empty. -fn is_dominant(next: &Atom, prev: &Atom) -> bool { - match next { - Atom::Empty => false, - Atom::Space => *prev == Atom::Empty, - Atom::Hardline => *prev == Atom::Space || *prev == Atom::Empty, - Atom::Blankline => *prev != Atom::Blankline, - _ => panic!("Unexpected character in is_dominant"), - } -} - /// Flatten the tree, depth-first, into a vector of nodes. /// /// This function takes a reference to a node and returns a vector of references @@ -1145,6 +1148,50 @@ mod test { ); } + #[test] + fn post_process_hardline_before_hardline() { + let mut atom_collection = AtomCollection::new(vec![ + Atom::Literal("foo".into()), + Atom::Hardline, + Atom::Hardline, + Atom::Literal("foo".into()), + ]); + + atom_collection.post_process(); + + assert_eq!( + atom_collection.atoms, + vec![ + Atom::Literal("foo".into()), + Atom::Hardline, + Atom::Empty, + Atom::Literal("foo".into()), + ] + ); + } + + #[test] + fn post_process_empty_blank_hard() { + let mut atom_collection = AtomCollection::new(vec![ + Atom::Empty, + Atom::Blankline, + Atom::Hardline, + Atom::Literal("foo".into()), + ]); + + atom_collection.post_process(); + + assert_eq!( + atom_collection.atoms, + vec![ + Atom::Empty, + Atom::Blankline, + Atom::Empty, + Atom::Literal("foo".into()), + ] + ); + } + #[test] fn issue_549_post_process_indent_before_hardline_with_antispace_in_between() { let mut atom_collection = AtomCollection::new(vec![ diff --git a/topiary-core/src/lib.rs b/topiary-core/src/lib.rs index 28509540..b0607886 100644 --- a/topiary-core/src/lib.rs +++ b/topiary-core/src/lib.rs @@ -114,6 +114,20 @@ pub enum Atom { }, } +impl Atom { + /// This function is only expected to take spaces and newlines as argument. + /// It defines the order Blankline > Hardline > Space > Empty. + pub(crate) fn dominates(&self, other: &Atom) -> bool { + match self { + Atom::Empty => false, + Atom::Space => matches!(other, Atom::Empty), + Atom::Hardline => matches!(other, Atom::Space | Atom::Empty), + Atom::Blankline => matches!(other, Atom::Hardline | Atom::Space | Atom::Empty), + _ => panic!("Unexpected character in is_dominant"), + } + } +} + /// Used in `Atom::ScopedConditional` to apply the containing Atoms only if /// the matched node spans a single line or multiple lines #[derive(Clone, Copy, Debug, Eq, PartialEq)]