From db64c83883e42089f96cf0bd1f253c100de44489 Mon Sep 17 00:00:00 2001 From: Lukasz Anforowicz Date: Wed, 5 Oct 2022 18:13:40 +0000 Subject: [PATCH] Handling of numbered markdown lists. Fixes issue #5416 --- src/comment.rs | 156 ++++++++++++++++++------ tests/source/itemized-blocks/no_wrap.rs | 36 +++++- tests/source/itemized-blocks/wrap.rs | 36 +++++- tests/target/itemized-blocks/no_wrap.rs | 36 +++++- tests/target/itemized-blocks/wrap.rs | 62 +++++++++- 5 files changed, 288 insertions(+), 38 deletions(-) diff --git a/src/comment.rs b/src/comment.rs index 4d565afc1e0..880ac630b69 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -432,7 +432,9 @@ impl CodeBlockAttribute { /// Block that is formatted as an item. /// -/// An item starts with either a star `*` a dash `-` or a greater-than `>`. +/// An item starts with either a star `*`, a dash `-`, a greater-than `>`, +/// or a number `12.` or `34)` (with at most 2 digits). +/// /// Different level of indentation are handled by shrinking the shape accordingly. struct ItemizedBlock { /// the lines that are identified as part of an itemized block @@ -446,36 +448,47 @@ struct ItemizedBlock { } impl ItemizedBlock { - /// Returns `true` if the line is formatted as an item - fn is_itemized_line(line: &str) -> bool { - let trimmed = line.trim_start(); - trimmed.starts_with("* ") || trimmed.starts_with("- ") || trimmed.starts_with("> ") - } - - /// Creates a new ItemizedBlock described with the given line. - /// The `is_itemized_line` needs to be called first. - fn new(line: &str) -> ItemizedBlock { - let space_to_sigil = line.chars().take_while(|c| c.is_whitespace()).count(); - // +2 = '* ', which will add the appropriate amount of whitespace to keep itemized - // content formatted correctly. - let mut indent = space_to_sigil + 2; - let mut line_start = " ".repeat(indent); - - // Markdown blockquote start with a "> " - if line.trim_start().starts_with(">") { - // remove the original +2 indent because there might be multiple nested block quotes - // and it's easier to reason about the final indent by just taking the length - // of th new line_start. We update the indent because it effects the max width - // of each formatted line. - line_start = itemized_block_quote_start(line, line_start, 2); - indent = line_start.len(); + /// Returns the sigil's (e.g. "- ", "* ", or "1. ") length or None if there is no sigil. + fn get_sigil_length(trimmed: &str) -> Option { + if trimmed.starts_with("* ") || trimmed.starts_with("- ") || trimmed.starts_with("> ") { + return Some(2); } - ItemizedBlock { - lines: vec![line[indent..].to_string()], - indent, - opener: line[..indent].to_string(), - line_start, + + for suffix in [". ", ") "] { + if let Some((prefix, _)) = trimmed.split_once(suffix) { + if prefix.len() <= 2 && prefix.chars().all(|c| char::is_ascii_digit(&c)) { + return Some(prefix.len() + suffix.len()); + } + } } + + None + } + + /// Creates a new ItemizedBlock described with the given `line` + /// or None if `line` doesn't start an item. + fn new(line: &str) -> Option { + ItemizedBlock::get_sigil_length(line.trim_start()).map(|sigil_length| { + let space_to_sigil = line.chars().take_while(|c| c.is_whitespace()).count(); + let mut indent = space_to_sigil + sigil_length; + let mut line_start = " ".repeat(indent); + + // Markdown blockquote start with a "> " + if line.trim_start().starts_with(">") { + // remove the original +2 indent because there might be multiple nested block quotes + // and it's easier to reason about the final indent by just taking the length + // of the new line_start. We update the indent because it effects the max width + // of each formatted line. + line_start = itemized_block_quote_start(line, line_start, 2); + indent = line_start.len(); + } + ItemizedBlock { + lines: vec![line[indent..].to_string()], + indent, + opener: line[..indent].to_string(), + line_start, + } + }) } /// Returns a `StringFormat` used for formatting the content of an item. @@ -494,7 +507,7 @@ impl ItemizedBlock { /// Returns `true` if the line is part of the current itemized block. /// If it is, then it is added to the internal lines list. fn add_line(&mut self, line: &str) -> bool { - if !ItemizedBlock::is_itemized_line(line) + if ItemizedBlock::get_sigil_length(line.trim_start()).is_none() && self.indent <= line.chars().take_while(|c| c.is_whitespace()).count() { self.lines.push(line.to_string()); @@ -765,10 +778,11 @@ impl<'a> CommentRewrite<'a> { self.item_block = None; if let Some(stripped) = line.strip_prefix("```") { self.code_block_attr = Some(CodeBlockAttribute::new(stripped)) - } else if self.fmt.config.wrap_comments() && ItemizedBlock::is_itemized_line(line) { - let ib = ItemizedBlock::new(line); - self.item_block = Some(ib); - return false; + } else if self.fmt.config.wrap_comments() { + if let Some(ib) = ItemizedBlock::new(line) { + self.item_block = Some(ib); + return false; + } } if self.result == self.opener { @@ -2004,4 +2018,78 @@ fn main() { "#; assert_eq!(s, filter_normal_code(s_with_comment)); } + + #[test] + fn test_itemized_block_first_line_handling() { + fn run_test( + test_input: &str, + expected_line: &str, + expected_indent: usize, + expected_opener: &str, + expected_line_start: &str, + ) { + let block = ItemizedBlock::new(test_input).unwrap(); + assert_eq!(1, block.lines.len(), "test_input: {:?}", test_input); + assert_eq!( + expected_line, &block.lines[0], + "test_input: {:?}", + test_input + ); + assert_eq!( + expected_indent, block.indent, + "test_input: {:?}", + test_input + ); + assert_eq!( + expected_opener, &block.opener, + "test_input: {:?}", + test_input + ); + assert_eq!( + expected_line_start, &block.line_start, + "test_input: {:?}", + test_input + ); + } + + run_test("- foo", "foo", 2, "- ", " "); + run_test("* foo", "foo", 2, "* ", " "); + run_test("> foo", "foo", 2, "> ", "> "); + + run_test("1. foo", "foo", 3, "1. ", " "); + run_test("12. foo", "foo", 4, "12. ", " "); + + run_test(" - foo", "foo", 6, " - ", " "); + } + + #[test] + fn test_itemized_block_nonobvious_sigils_are_rejected() { + let test_inputs = vec![ + // Non-numeric sigils (e.g. `a.` or `iv.`) are not supported, because of a risk of + // misidentifying regular words as sigils. See also the discussion in + // https://talk.commonmark.org/t/blank-lines-before-lists-revisited/1990 + "word. rest of the paragraph.", + "a. maybe this is a list item? maybe not?", + "iv. maybe this is a list item? maybe not?", + // Numbers with 3 or more digits are not recognized as sigils, to avoid + // formatting the following example as a list: + // + // ``` + // The Captain died in + // 1868. He was buried in... + // ``` + "123. only 2-digit numbers are recognized as sigils.", + // Parens. + "123) giving some coverage to parens as well.", + "a) giving some coverage to parens as well.", + ]; + for line in test_inputs.iter() { + let maybe_block = ItemizedBlock::new(line); + assert!( + maybe_block.is_none(), + "The following line shouldn't be classified as a list item: {}", + line + ); + } + } } diff --git a/tests/source/itemized-blocks/no_wrap.rs b/tests/source/itemized-blocks/no_wrap.rs index a7b6a10a010..e5699e76684 100644 --- a/tests/source/itemized-blocks/no_wrap.rs +++ b/tests/source/itemized-blocks/no_wrap.rs @@ -1,7 +1,7 @@ // rustfmt-normalize_comments: true // rustfmt-format_code_in_doc_comments: true -//! This is a list: +//! This is an itemized markdown list (see also issue #3224): //! * Outer //! * Outer //! * Inner @@ -13,6 +13,40 @@ //! - when the log level is info, the level name is green and the rest of the line is white //! - when the log level is debug, the whole line is white //! - when the log level is trace, the whole line is gray ("bright black") +//! +//! This is a numbered markdown list (see also issue #5416): +//! 1. Long long long long long long long long long long long long long long long long long line +//! 2. Another very long long long long long long long long long long long long long long long line +//! 3. Nested list +//! 1. Long long long long long long long long long long long long long long long long line +//! 2. Another very long long long long long long long long long long long long long long line +//! 4. Last item +//! +//! Using the ')' instead of '.' character after the number: +//! 1) Long long long long long long long long long long long long long long long long long line +//! 2) Another very long long long long long long long long long long long long long long long line +//! +//! Deep list that mixes various bullet and number formats: +//! 1. First level with a long long long long long long long long long long long long long long +//! long long long line +//! 2. First level with another very long long long long long long long long long long long long +//! long long long line +//! * Second level with a long long long long long long long long long long long long long +//! long long long line +//! * Second level with another very long long long long long long long long long long long +//! long long long line +//! 1) Third level with a long long long long long long long long long long long long long +//! long long long line +//! 2) Third level with another very long long long long long long long long long long +//! long long long long line +//! - Forth level with a long long long long long long long long long long long long +//! long long long long line +//! - Forth level with another very long long long long long long long long long long +//! long long long long line +//! 3) One more item at the third level +//! 4) Last item of the third level +//! * Last item of second level +//! 3. Last item of first level /// All the parameters ***except for `from_theater`*** should be inserted as sent by the remote /// theater, i.e., as passed to [`Theater::send`] on the remote actor: diff --git a/tests/source/itemized-blocks/wrap.rs b/tests/source/itemized-blocks/wrap.rs index 955cc698b79..768461a43f9 100644 --- a/tests/source/itemized-blocks/wrap.rs +++ b/tests/source/itemized-blocks/wrap.rs @@ -2,7 +2,7 @@ // rustfmt-format_code_in_doc_comments: true // rustfmt-max_width: 50 -//! This is a list: +//! This is an itemized markdown list (see also issue #3224): //! * Outer //! * Outer //! * Inner @@ -14,6 +14,40 @@ //! - when the log level is info, the level name is green and the rest of the line is white //! - when the log level is debug, the whole line is white //! - when the log level is trace, the whole line is gray ("bright black") +//! +//! This is a numbered markdown list (see also issue #5416): +//! 1. Long long long long long long long long long long long long long long long long long line +//! 2. Another very long long long long long long long long long long long long long long long line +//! 3. Nested list +//! 1. Long long long long long long long long long long long long long long long long line +//! 2. Another very long long long long long long long long long long long long long long line +//! 4. Last item +//! +//! Using the ')' instead of '.' character after the number: +//! 1) Long long long long long long long long long long long long long long long long long line +//! 2) Another very long long long long long long long long long long long long long long long line +//! +//! Deep list that mixes various bullet and number formats: +//! 1. First level with a long long long long long long long long long long long long long long +//! long long long line +//! 2. First level with another very long long long long long long long long long long long long +//! long long long line +//! * Second level with a long long long long long long long long long long long long long +//! long long long line +//! * Second level with another very long long long long long long long long long long long +//! long long long line +//! 1) Third level with a long long long long long long long long long long long long long +//! long long long line +//! 2) Third level with another very long long long long long long long long long long +//! long long long long line +//! - Forth level with a long long long long long long long long long long long long +//! long long long long line +//! - Forth level with another very long long long long long long long long long long +//! long long long long line +//! 3) One more item at the third level +//! 4) Last item of the third level +//! * Last item of second level +//! 3. Last item of first level // This example shows how to configure fern to output really nicely colored logs // - when the log level is error, the whole line is red diff --git a/tests/target/itemized-blocks/no_wrap.rs b/tests/target/itemized-blocks/no_wrap.rs index de885638272..86818b44745 100644 --- a/tests/target/itemized-blocks/no_wrap.rs +++ b/tests/target/itemized-blocks/no_wrap.rs @@ -1,7 +1,7 @@ // rustfmt-normalize_comments: true // rustfmt-format_code_in_doc_comments: true -//! This is a list: +//! This is an itemized markdown list (see also issue #3224): //! * Outer //! * Outer //! * Inner @@ -13,6 +13,40 @@ //! - when the log level is info, the level name is green and the rest of the line is white //! - when the log level is debug, the whole line is white //! - when the log level is trace, the whole line is gray ("bright black") +//! +//! This is a numbered markdown list (see also issue #5416): +//! 1. Long long long long long long long long long long long long long long long long long line +//! 2. Another very long long long long long long long long long long long long long long long line +//! 3. Nested list +//! 1. Long long long long long long long long long long long long long long long long line +//! 2. Another very long long long long long long long long long long long long long long line +//! 4. Last item +//! +//! Using the ')' instead of '.' character after the number: +//! 1) Long long long long long long long long long long long long long long long long long line +//! 2) Another very long long long long long long long long long long long long long long long line +//! +//! Deep list that mixes various bullet and number formats: +//! 1. First level with a long long long long long long long long long long long long long long +//! long long long line +//! 2. First level with another very long long long long long long long long long long long long +//! long long long line +//! * Second level with a long long long long long long long long long long long long long +//! long long long line +//! * Second level with another very long long long long long long long long long long long +//! long long long line +//! 1) Third level with a long long long long long long long long long long long long long +//! long long long line +//! 2) Third level with another very long long long long long long long long long long +//! long long long long line +//! - Forth level with a long long long long long long long long long long long long +//! long long long long line +//! - Forth level with another very long long long long long long long long long long +//! long long long long line +//! 3) One more item at the third level +//! 4) Last item of the third level +//! * Last item of second level +//! 3. Last item of first level /// All the parameters ***except for `from_theater`*** should be inserted as sent by the remote /// theater, i.e., as passed to [`Theater::send`] on the remote actor: diff --git a/tests/target/itemized-blocks/wrap.rs b/tests/target/itemized-blocks/wrap.rs index a4907303c9e..4826590ea59 100644 --- a/tests/target/itemized-blocks/wrap.rs +++ b/tests/target/itemized-blocks/wrap.rs @@ -2,7 +2,8 @@ // rustfmt-format_code_in_doc_comments: true // rustfmt-max_width: 50 -//! This is a list: +//! This is an itemized markdown list (see also +//! issue #3224): //! * Outer //! * Outer //! * Inner @@ -23,6 +24,65 @@ //! is white //! - when the log level is trace, the whole line //! is gray ("bright black") +//! +//! This is a numbered markdown list (see also +//! issue #5416): +//! 1. Long long long long long long long long +//! long long long long long long long long +//! long line +//! 2. Another very long long long long long long +//! long long long long long long long long +//! long line +//! 3. Nested list +//! 1. Long long long long long long long long +//! long long long long long long long long +//! line +//! 2. Another very long long long long long +//! long long long long long long long long +//! long line +//! 4. Last item +//! +//! Using the ')' instead of '.' character after +//! the number: +//! 1) Long long long long long long long long +//! long long long long long long long long +//! long line +//! 2) Another very long long long long long long +//! long long long long long long long long +//! long line +//! +//! Deep list that mixes various bullet and number +//! formats: +//! 1. First level with a long long long long long +//! long long long long long long long long +//! long long long long line +//! 2. First level with another very long long +//! long long long long long long long long +//! long long long long long line +//! * Second level with a long long long long +//! long long long long long long long long +//! long long long long line +//! * Second level with another very long long +//! long long long long long long long long +//! long long long long line +//! 1) Third level with a long long long +//! long long long long long long long +//! long long long long long long line +//! 2) Third level with another very long +//! long long long long long long long +//! long long long long long long line +//! - Forth level with a long long +//! long long long long long long +//! long long long long long long +//! long long line +//! - Forth level with another very +//! long long long long long long +//! long long long long long long +//! long long line +//! 3) One more item at the third level +//! 4) Last item of the third level +//! * Last item of second level +//! 3. Last item of first level // This example shows how to configure fern to // output really nicely colored logs