From 74e25c11f1962c8d0615523b9aa1f069ebc04bd4 Mon Sep 17 00:00:00 2001 From: Daniel Eichman <61132910+zfz7@users.noreply.github.com> Date: Mon, 21 Oct 2024 06:57:49 -0700 Subject: [PATCH] Fix incorrect checkbox placement in Markdown preview (#19383) - Closes #12515 Before fix: Screenshot 2024-10-17 at 09 50 19 After fix: Screenshot 2024-10-17 at 09 52 36 Testing: - Manual testing - Added unit test Test results, these tests fail on the main branch for my setup as well, I have docker running but still had some failures: ``` failures: tests::integration_tests::test_context_collaboration_with_reconnect tests::integration_tests::test_formatting_buffer tests::integration_tests::test_fs_operations tests::integration_tests::test_git_branch_name tests::integration_tests::test_git_diff_base_change tests::integration_tests::test_git_status_sync tests::integration_tests::test_join_after_restart tests::integration_tests::test_join_call_after_screen_was_shared tests::integration_tests::test_joining_channels_and_calling_multiple_users_simultaneously tests::integration_tests::test_leaving_project tests::integration_tests::test_leaving_worktree_while_opening_buffer tests::integration_tests::test_local_settings tests::integration_tests::test_lsp_hover tests::integration_tests::test_mute_deafen tests::integration_tests::test_open_buffer_while_getting_definition_pointing_to_it tests::integration_tests::test_pane_split_left tests::integration_tests::test_prettier_formatting_buffer tests::integration_tests::test_preview_tabs tests::integration_tests::test_project_reconnect tests::integration_tests::test_project_search tests::integration_tests::test_project_symbols tests::integration_tests::test_propagate_saves_and_fs_changes tests::integration_tests::test_references tests::integration_tests::test_reloading_buffer_manually tests::integration_tests::test_right_click_menu_behind_collab_panel tests::integration_tests::test_room_location tests::integration_tests::test_room_uniqueness tests::integration_tests::test_server_restarts tests::integration_tests::test_unshare_project tests::notification_tests::test_notifications tests::random_project_collaboration_tests::test_random_project_collaboration tests::remote_editing_collaboration_tests::test_sharing_an_ssh_remote_project test result: FAILED. 156 passed; 32 failed; 0 ignored; 0 measured; 0 filtered out; finished in 100.98s ``` Comments: I do not have a ton of rust knowledge, so very open to feedback. TYSM Release Notes: - Fix Incorrect checkbox placement in Markdown preview --------- Co-authored-by: Bennet Bo Fenner --- .../markdown_preview/src/markdown_parser.rs | 84 ++++++++++++++----- 1 file changed, 61 insertions(+), 23 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 9580e9b962..10e910036b 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -36,6 +36,20 @@ struct MarkdownParser<'a> { language_registry: Option>, } +struct MarkdownListItem { + content: Vec, + item_type: ParsedMarkdownListItemType, +} + +impl Default for MarkdownListItem { + fn default() -> Self { + Self { + content: Vec::new(), + item_type: ParsedMarkdownListItemType::Unordered, + } + } +} + impl<'a> MarkdownParser<'a> { fn new( tokens: Vec<(Event<'a>, Range)>, @@ -475,9 +489,8 @@ impl<'a> MarkdownParser<'a> { let (_, list_source_range) = self.previous().unwrap(); let mut items = Vec::new(); - let mut items_stack = vec![Vec::new()]; + let mut items_stack = vec![MarkdownListItem::default()]; let mut depth = 1; - let mut task_item = None; let mut order = order; let mut order_stack = Vec::new(); @@ -517,8 +530,9 @@ impl<'a> MarkdownParser<'a> { start_item_range = source_range.clone(); self.cursor += 1; - items_stack.push(Vec::new()); + items_stack.push(MarkdownListItem::default()); + let mut task_list = None; // Check for task list marker (`- [ ]` or `- [x]`) if let Some(event) = self.current_event() { // If there is a linebreak in between two list items the task list marker will actually be the first element of the paragraph @@ -527,7 +541,7 @@ impl<'a> MarkdownParser<'a> { } if let Some((Event::TaskListMarker(checked), range)) = self.current() { - task_item = Some((*checked, range.clone())); + task_list = Some((*checked, range.clone())); self.cursor += 1; } } @@ -539,13 +553,21 @@ impl<'a> MarkdownParser<'a> { let text = self.parse_text(false, Some(range.clone())); let block = ParsedMarkdownElement::Paragraph(text); if let Some(content) = items_stack.last_mut() { - content.push(block); + let item_type = if let Some((checked, range)) = task_list { + ParsedMarkdownListItemType::Task(checked, range) + } else if let Some(order) = order { + ParsedMarkdownListItemType::Ordered(order) + } else { + ParsedMarkdownListItemType::Unordered + }; + content.item_type = item_type; + content.content.push(block); } } else { let block = self.parse_block().await; if let Some(block) = block { - if let Some(content) = items_stack.last_mut() { - content.extend(block); + if let Some(list_item) = items_stack.last_mut() { + list_item.content.extend(block); } } } @@ -559,19 +581,11 @@ impl<'a> MarkdownParser<'a> { Event::End(TagEnd::Item) => { self.cursor += 1; - let item_type = if let Some((checked, range)) = task_item { - ParsedMarkdownListItemType::Task(checked, range) - } else if let Some(order) = order { - ParsedMarkdownListItemType::Ordered(order) - } else { - ParsedMarkdownListItemType::Unordered - }; - if let Some(current) = order { order = Some(current + 1); } - if let Some(content) = items_stack.pop() { + if let Some(list_item) = items_stack.pop() { let source_range = source_ranges .remove(&depth) .unwrap_or(start_item_range.clone()); @@ -580,9 +594,9 @@ impl<'a> MarkdownParser<'a> { let source_range = source_range.start..source_range.end - 1; let item = ParsedMarkdownElement::ListItem(ParsedMarkdownListItem { source_range, - content, + content: list_item.content, depth, - item_type, + item_type: list_item.item_type, }); if let Some(index) = insertion_indices.get(&depth) { @@ -592,8 +606,6 @@ impl<'a> MarkdownParser<'a> { items.push(item); } } - - task_item = None; } _ => { if depth == 0 { @@ -603,10 +615,10 @@ impl<'a> MarkdownParser<'a> { // or the list item contains blocks that should be rendered after the nested list items let block = self.parse_block().await; if let Some(block) = block { - if let Some(items_stack) = items_stack.last_mut() { + if let Some(list_item) = items_stack.last_mut() { // If we did not insert any nested items yet (in this case insertion index is set), we can append the block to the current list item if !insertion_indices.contains_key(&depth) { - items_stack.extend(block); + list_item.content.extend(block); continue; } } @@ -722,7 +734,6 @@ mod tests { use gpui::BackgroundExecutor; use language::{tree_sitter_rust, HighlightId, Language, LanguageConfig, LanguageMatcher}; use pretty_assertions::assert_eq; - use ParsedMarkdownListItemType::*; async fn parse(input: &str) -> ParsedMarkdown { @@ -956,6 +967,33 @@ Some other content ); } + #[gpui::test] + async fn test_list_with_indented_task() { + let parsed = parse( + "\ +- [ ] TODO + - [x] Checked + - Unordered + 1. Number 1 + 1. Number 2 +1. Number A +", + ) + .await; + + assert_eq!( + parsed.children, + vec![ + list_item(0..12, 1, Task(false, 2..5), vec![p("TODO", 6..10)]), + list_item(13..26, 2, Task(true, 15..18), vec![p("Checked", 19..26)]), + list_item(29..40, 2, Unordered, vec![p("Unordered", 31..40)]), + list_item(43..54, 2, Ordered(1), vec![p("Number 1", 46..54)]), + list_item(57..68, 2, Ordered(2), vec![p("Number 2", 60..68)]), + list_item(69..80, 1, Ordered(1), vec![p("Number A", 72..80)]), + ], + ); + } + #[gpui::test] async fn test_list_with_linebreak_is_handled_correctly() { let parsed = parse(