Skip to content

Commit

Permalink
fix: use slice patterns over iterator
Browse files Browse the repository at this point in the history
  • Loading branch information
Erin van der Veen committed May 30, 2024
1 parent 82a94a9 commit d67a363
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 69 deletions.
185 changes: 116 additions & 69 deletions topiary-core/src/atom_collection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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![
Expand Down
14 changes: 14 additions & 0 deletions topiary-core/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down

0 comments on commit d67a363

Please sign in to comment.