Skip to content

Commit

Permalink
markdown rendering tests and fixes
Browse files Browse the repository at this point in the history
- Add a more generic version of the variants checker I wrote for the
   Node -> MdqNode conversions
- Add unit tests for all the MdqNode -> markdown renderings
- Various fixes to the prod code that shook out from those tests
  • Loading branch information
yshavit authored Jun 16, 2024
1 parent c13284b commit 5e69316
Show file tree
Hide file tree
Showing 7 changed files with 1,579 additions and 100 deletions.
1,470 changes: 1,400 additions & 70 deletions src/fmt_md.rs

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ mod output;
mod select;
mod str_utils;
mod tree;
mod tree_test_utils;
mod utils_for_test;

fn main() {
let mut contents = String::new();
Expand Down
55 changes: 42 additions & 13 deletions src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,10 @@ impl<W: SimpleWrite> Output<W> {
}

// print the newlines before we append the new blocks
let need_indent = if self.pending_newlines > 0 {
let need_full_indent = if self.pending_newlines > 0 {
for _ in 0..(self.pending_newlines - 1) {
self.write_raw("\n");
self.write_indent(None);
self.write_indent(true, None);
}
self.write_raw("\n"); // we'll do this indent after we add the pending blocks
self.pending_newlines = 0;
Expand All @@ -167,15 +167,20 @@ impl<W: SimpleWrite> Output<W> {

// Append the new blocks, and then write the indent if we need it
// When we write that indent, though, only write it until the first new Inlined (exclusive).
let indent_end_idx = self.blocks.len()
+ self
.pending_blocks
.iter()
.position(|b| matches!(b, Block::Inlined(_)))
.unwrap_or_else(|| self.pending_blocks.len());
self.blocks.append(&mut self.pending_blocks);
if need_indent {
self.write_indent(Some(0..indent_end_idx));
if need_full_indent {
let indent_end_idx = self.blocks.len()
+ self
.pending_blocks
.iter()
.position(|b| matches!(b, Block::Inlined(_)))
.unwrap_or_else(|| self.pending_blocks.len());
self.blocks.append(&mut self.pending_blocks);
self.write_indent(true, Some(0..indent_end_idx));
} else {
self.write_padding_after_indent();
let prev_blocks_len = self.blocks.len();
self.blocks.append(&mut self.pending_blocks);
self.write_indent(false, Some(prev_blocks_len..self.blocks.len()));
}

// Finally, write the text
Expand All @@ -185,7 +190,9 @@ impl<W: SimpleWrite> Output<W> {
self.write_raw(text);
}

fn write_indent(&mut self, range: Option<Range<usize>>) {
/// Write an indentation for a given range of indentation block. If `include_inlines` is false, `Inlined` blocks
/// will be disregarded. Do this for the first line in which those inlines appear (and only there).
fn write_indent(&mut self, include_inlines: bool, range: Option<Range<usize>>) {
self.pending_padding_after_indent = 0;
for idx in range.unwrap_or_else(|| 0..self.blocks.len()) {
match &self.blocks[idx] {
Expand All @@ -196,7 +203,9 @@ impl<W: SimpleWrite> Output<W> {
self.pending_padding_after_indent += 1;
}
Block::Inlined(size) => {
self.pending_padding_after_indent += size;
if include_inlines {
self.pending_padding_after_indent += size;
}
}
}
}
Expand Down Expand Up @@ -239,6 +248,9 @@ impl<W: SimpleWrite> Output<W> {
eprintln!("error while writing output: {}", e);
self.writing_state = WritingState::Error;
}
if matches!(self.writing_state, WritingState::HaveNotWrittenAnything) {
self.writing_state = WritingState::Normal;
}
}
}

Expand Down Expand Up @@ -357,6 +369,23 @@ mod tests {
);
}

#[test]
fn quote_block_one_char_at_a_time_as_initial_output() {
// We haven't written anything, and then we start writing quote blocks
assert_eq!(
out_to_str(|out| {
out.with_block(Block::Quote, |out| {
out.write_str(""); // empty prefix char
"hello\nworld".chars().for_each(|c| out.write_char(c));
out.write_str(""); // empty suffix char
});
}),
indoc! {r#"
> hello
> world"#}
);
}

#[test]
fn inlined_block() {
assert_eq!(
Expand Down
30 changes: 14 additions & 16 deletions src/str_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,59 +17,57 @@ where
let padding = min_width - input.len();

match standard_align(alignment) {
Alignment::Left => {
Some(Alignment::Left) | None => {
output.write_str(input);
(0..padding).for_each(|_| output.write_char(' '));
}
Alignment::Center => {
Some(Alignment::Center) => {
let left_pad = padding / 2; // round down
let right_pad = padding - left_pad;
(0..left_pad).for_each(|_| output.write_char(' '));
output.write_str(input);
(0..right_pad).for_each(|_| output.write_char(' '));
}
Alignment::Right => {
Some(Alignment::Right) => {
(0..padding).for_each(|_| output.write_char(' '));
output.write_str(input);
}
}
}

pub fn standard_align<A>(mdast_align: A) -> Alignment
pub fn standard_align<A>(mdast_align: A) -> Option<Alignment>
where
A: ToAlignment,
{
mdast_align.to_alignment()
}

const DEFAULT_ALIGNMENT: Alignment = Alignment::Left;

pub trait ToAlignment {
fn to_alignment(&self) -> Alignment;
fn to_alignment(&self) -> Option<Alignment>;
}

impl ToAlignment for Alignment {
fn to_alignment(&self) -> Alignment {
*self
fn to_alignment(&self) -> Option<Alignment> {
Some(*self)
}
}

impl ToAlignment for &AlignKind {
fn to_alignment(&self) -> Alignment {
fn to_alignment(&self) -> Option<Alignment> {
match self {
AlignKind::Left => Alignment::Left,
AlignKind::Right => Alignment::Right,
AlignKind::Center => Alignment::Center,
_ => DEFAULT_ALIGNMENT,
AlignKind::Left => Some(Alignment::Left),
AlignKind::Right => Some(Alignment::Right),
AlignKind::Center => Some(Alignment::Center),
AlignKind::None => None,
}
}
}

impl<A: Borrow<AlignKind>> ToAlignment for Option<A> {
fn to_alignment(&self) -> Alignment {
fn to_alignment(&self) -> Option<Alignment> {
match self {
Some(a) => a.borrow().to_alignment(),
None => DEFAULT_ALIGNMENT,
None => None,
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub enum MdqNode {
CodeBlock(CodeBlock),

// inline spans
Inline(Inline), // TODO rename to "span"?
Inline(Inline),
}

#[derive(Debug, PartialEq)]
Expand Down
47 changes: 47 additions & 0 deletions src/tree_test_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#[cfg(test)]
mod test_utils {

#[macro_export]
macro_rules! mdq_node {
($node_type:tt {$($attr:ident: $val:expr),*}) => {
MdqNode::$node_type($node_type{$($attr: $val),*})
};
($paragraph_text:literal) => {
crate::mdq_node!(Paragraph{body: vec![crate::mdq_inline!($paragraph_text)]})
};
}

#[macro_export]
macro_rules! mdq_nodes {
[$($node_type:tt {$($attr:ident: $val:expr),*$(,)?}),*$(,)?] => {
vec![$(
crate::mdq_node!($node_type {
$($attr: $val),*
})
),*
]
};
[$($paragraph_text:literal),*$(,)?] => {
vec![$(
crate::mdq_node!($paragraph_text)
),*
]
}
}

#[macro_export]
macro_rules! mdq_inline {
(span $which:ident [$($contents:expr),*$(,)?]) => {
crate::tree::Inline::Span {
variant: crate::tree::SpanVariant::$which,
children: vec![$($contents),*],
}
};
($text:literal) => {
crate::tree::Inline::Text {
variant: crate::tree::InlineVariant::Text,
value: $text.to_string(),
}
};
}
}
73 changes: 73 additions & 0 deletions src/utils_for_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
#[cfg(test)]
pub use test_utils::*;

// We this file's contents from prod by putting them in a submodule guarded by cfg(test), but then "pub use" it to
// export its contents.
#[cfg(test)]
mod test_utils {
use std::{thread, time};

// TODO unify this with the one in tree.rs.
pub struct VariantsChecker<E> {
require: std::sync::Arc<std::sync::Mutex<std::collections::HashSet<String>>>,
resolver: fn(&E) -> &str,
}

impl<E> VariantsChecker<E> {
pub fn new<I>(require: I, resolver: fn(&E) -> &str) -> Self
where
I: IntoIterator<Item = String>,
{
Self {
require: std::sync::Arc::new(std::sync::Mutex::new(require.into_iter().collect())),
resolver,
}
}

pub fn see(&self, node: &E) {
let node_str = (self.resolver)(node);
self.require.lock().map(|mut set| set.remove(node_str)).unwrap();
}

pub fn wait_for_all(&self) {
let timeout = time::Duration::from_millis(500);
let retry_delay = time::Duration::from_millis(50);
let start = time::Instant::now();
loop {
if self.require.lock().map(|set| set.is_empty()).unwrap() {
break;
}
if start.elapsed() >= timeout {
let mut remaining: Vec<String> = self
.require
.lock()
.map(|set| set.iter().map(|s| s.to_owned()).collect())
.unwrap();
remaining.sort();
panic!(
"Timed out, and missing {} variants:\n- {}",
remaining.len(),
remaining.join("\n- ")
)
}
thread::sleep(retry_delay);
}
}
}

#[macro_export]
macro_rules! new_variants_checker {
{$enum_type:ty : $($variant:pat),* $(,)?} => {
{
use $enum_type::*;

VariantsChecker::new(
vec![$(stringify!($variant).to_string(),)*],
{|elem| match elem {
$($variant => stringify!($variant),)*
}}
)
}
};
}
}

0 comments on commit 5e69316

Please sign in to comment.