Group plaintext lines in comment editors

app/src/code/editor/comment_editor.rs

@@ -543,6 +543,7 @@ where
                 can_execute_shell_commands: Some(false),
                 disable_block_insertion_menu: true,
                 disable_scrolling,
+                group_plain_text_lines: true,
             },
             ctx,
         )

app/src/code/editor/comment_editor_tests.rs

@@ -9,6 +9,10 @@ use super::{create_editable_comment_markdown_editor, create_readonly_comment_mar
 use crate::notebooks::editor::view::RichTextEditorView;
 use repo_metadata::repositories::DetectedRepositories;
 use repo_metadata::RepoMetadataModel;
+use warp_editor::{
+    model::{CoreEditorModel, RichTextEditorModel},
+    render::model::BlockItem,
+};
 
 use crate::{
     appearance::Appearance,
@@ -138,6 +142,41 @@ fn test_editable_comment_editor_keeps_final_trailing_newline_for_non_empty_code_
     });
 }
 
+#[test]
+fn test_comment_editor_groups_adjacent_plain_text_lines() {
+    App::test((), |mut app| async move {
+        let (_window, editor_view, _test_view) =
+            initialize_editor(&mut app, CommentEditorMode::Editable);
+        let render_model = editor_view.read(&app, |editor, ctx| {
+            editor.model().as_ref(ctx).render_state().clone()
+        });
+
+        editor_view.update(&mut app, |editor, ctx| {
+            editor.model().update(ctx, |model, ctx| {
+                model.user_insert("first plaintext line", ctx);
+                model.enter(ctx);
+                model.user_insert("second plaintext line", ctx);
+            });
+        });
+
+        let pending_layout =
+            render_model.read(&app, |render_state, _| render_state.layout_complete());
+        pending_layout.await;
+
+        render_model.read(&app, |render_state, _| {
+            let content = render_state.content();
+            let items = content.block_items().collect::<Vec<_>>();
+            assert_eq!(items.len(), 1);
+            match items[0] {
+                BlockItem::TextBlock { paragraph_block } => {
+                    assert_eq!(paragraph_block.paragraphs().len(), 2);
+                }
+                other => panic!("expected grouped text block, got {other:?}"),
+            }
+        });
+    });
+}
+
 #[test]
 fn test_readonly_comment_editor_hides_final_trailing_newline_for_non_empty_code_blocks() {
     App::test((), |mut app| async move {

app/src/notebooks/editor/view.rs

@@ -1105,6 +1105,10 @@ pub struct RichTextEditorConfig {
     /// Enable or disable the block insertion menu (slash menu).
     /// When disabled, typing "/" will not open the menu.
     pub disable_block_insertion_menu: bool,
+
+    /// When true, adjacent plain-text lines are laid out as one text block instead of separate
+    /// paragraph blocks. This avoids inter-paragraph spacing for compact editors such as comments.
+    pub group_plain_text_lines: bool,
 }
 
 impl RichTextEditorView {
@@ -1159,6 +1163,15 @@ impl RichTextEditorView {
 
         let insertion_menu_state =
             BlockInsertionMenuState::new(ctx, config.embedded_objects_enabled.unwrap_or(true));
+        if config.group_plain_text_lines {
+            model
+                .as_ref(ctx)
+                .render_state()
+                .clone()
+                .update(ctx, |render_state, _| {
+                    render_state.set_group_plain_text_lines(true);
+                });
+        }
 
         Self {
             omnibar,

crates/editor/src/content/edit.rs

@@ -515,10 +515,14 @@ impl EditDelta {
 
         // old_offset is in the same 1-indexed coordinate system as hidden ranges.
         let mut current_offset = (self.old_offset.start).max(CharOffset::from(1));
+        let new_lines = if layout_options.group_plain_text_lines {
+            group_adjacent_plain_text_blocks(self.new_lines)
+        } else {
+            self.new_lines
+        };
 
         // First, build a Vec of layout tasks with information about whether they're hidden
-        let layout_tasks: Vec<_> = self
-            .new_lines
+        let layout_tasks: Vec<_> = new_lines
             .into_iter()
             .filter_map(|block| {
                 let content_length = block.content_length();
@@ -610,6 +614,31 @@ impl EditDelta {
     }
 }
 
+fn group_adjacent_plain_text_blocks(blocks: Vec<StyledBufferBlock>) -> Vec<StyledBufferBlock> {
+    let mut grouped_blocks = Vec::with_capacity(blocks.len());
+
+    for block in blocks {
+        match block {
+            StyledBufferBlock::Text(text_block)
+                if text_block.style == BufferBlockStyle::PlainText =>
+            {
+                if let Some(StyledBufferBlock::Text(previous_text_block)) =
+                    grouped_blocks.last_mut()
+                    && previous_text_block.style == BufferBlockStyle::PlainText
+                {
+                    previous_text_block.content_length += text_block.content_length;
+                    previous_text_block.block.extend(text_block.block);
+                    continue;
+                }
+
+                grouped_blocks.push(StyledBufferBlock::Text(text_block));
+            }
+            block => grouped_blocks.push(block),
+        }
+    }
+
+    grouped_blocks
+}
 /// Lay out a list of temporary blocks in parallel.
 pub fn layout_temporary_blocks(
     blocks: Vec<TemporaryBlock>,
@@ -662,7 +691,10 @@ enum LayoutTask {
     /// [`AppContext`].
     Embed(Box<dyn LaidOutEmbeddedItem>),
     /// A text block, which will be laid out in parallel.
-    Text(StyledTextBlock),
+    Text {
+        text_block: StyledTextBlock,
+        group_plain_text_lines: bool,
+    },
     MermaidDiagram {
         text_block: StyledTextBlock,
         asset_source: AssetSource,
@@ -842,7 +874,10 @@ impl LayoutTask {
                         }
                     }
                 } else {
-                    Self::Text(text_block)
+                    Self::Text {
+                        text_block,
+                        group_plain_text_lines: layout_options.group_plain_text_lines,
+                    }
                 }
             }
         }
@@ -888,13 +923,22 @@ impl LayoutTask {
                     true, // Images are always followed by a trailing newline in the buffer
                 ))
             }
-            Self::Text(text_block) => layout_text_block(text_block, layout, location, is_hidden),
+            Self::Text {
+                text_block,
+                group_plain_text_lines,
+            } => layout_text_block(
+                text_block,
+                layout,
+                location,
+                is_hidden,
+                group_plain_text_lines,
+            ),
             Self::MermaidCodeFallback {
                 text_block,
                 pending_mermaid_asset,
             } => {
                 let (block_item, has_trailing_newline) =
-                    layout_text_block(text_block, layout, location, is_hidden)?;
+                    layout_text_block(text_block, layout, location, is_hidden, false)?;
                 let block_item = match block_item {
                     BlockItem::RunnableCodeBlock {
                         paragraph_block,
@@ -941,6 +985,8 @@ fn estimate_paragraph_count(text_block: &StyledTextBlock) -> usize {
         // highlighting). Use the number of individual runs as an overestimate for the number
         // of lines.
         text_block.block.len()
+    } else if text_block.style == BufferBlockStyle::PlainText {
+        text_block.block.len()
     } else {
         // Non-code blocks may only contain a single paragraph.
         1
@@ -970,6 +1016,7 @@ fn layout_text_block(
     layout: &TextLayout,
     location: BlockLocation,
     is_hidden: bool,
+    group_plain_text_lines: bool,
 ) -> Result<(BlockItem, bool)> {
     if is_hidden {
         // If all text is hidden, return a BlockItem::Hidden without doing any layout
@@ -1139,11 +1186,21 @@ fn layout_text_block(
                 .ok_or_else(|| anyhow!("Header item should have one paragraph"))
         }
         BufferBlockStyle::PlainText => {
-            debug_assert_eq!(
-                paragraphs.len(),
-                1,
-                "Plain text paragraphs should only have one line."
-            );
+            if group_plain_text_lines && paragraphs.len() > 1 {
+                return Vec1::try_from_vec(paragraphs)
+                    .ok()
+                    .map(|paragraphs| BlockItem::TextBlock {
+                        paragraph_block: ParagraphBlock::new(paragraphs),
+                    })
+                    .ok_or_else(|| anyhow!("Plain text block should have at least one paragraph"))
+                    .map(|item| (item, has_trailing_newline));
+            } else {
+                debug_assert_eq!(
+                    paragraphs.len(),
+                    1,
+                    "Plain text paragraphs should only have one line."
+                );
+            }
 
             paragraphs
                 .pop()

crates/editor/src/content/edit_tests.rs

@@ -83,6 +83,49 @@ fn test_highlight_urls() {
     );
 }
 
+#[test]
+fn test_layout_text_block_groups_plain_text_lines_when_enabled() {
+    App::test((), |app| async move {
+        app.read(|ctx| {
+            let layout_cache = LayoutCache::new();
+            let text_layout = TextLayout::new(
+                &layout_cache,
+                ctx.font_cache().text_layout_system(),
+                &TEST_STYLES,
+                f32::MAX,
+            );
+            let block = StyledTextBlock {
+                block: vec![
+                    StyledBufferRun {
+                        run: "first\n".to_string(),
+                        text_styles: TextStylesWithMetadata::default(),
+                        block_style: BufferBlockStyle::PlainText,
+                    },
+                    StyledBufferRun {
+                        run: "second".to_string(),
+                        text_styles: TextStylesWithMetadata::default(),
+                        block_style: BufferBlockStyle::PlainText,
+                    },
+                ],
+                style: BufferBlockStyle::PlainText,
+                content_length: CharOffset::from("first\nsecond".chars().count()),
+            };
+
+            let (item, has_trailing_newline) =
+                layout_text_block(block, &text_layout, BlockLocation::Middle, false, true)
+                    .expect("plain text layout should succeed");
+
+            match item {
+                BlockItem::TextBlock { paragraph_block } => {
+                    assert_eq!(paragraph_block.paragraphs().len(), 2);
+                }
+                other => panic!("expected grouped text block, got {other:?}"),
+            }
+            assert!(!has_trailing_newline);
+        });
+    })
+}
+
 #[test]
 fn test_highlight_urls_unicode() {
     let test_runs = vec![StyledBufferRun {
@@ -693,7 +736,7 @@ fn test_layout_text_block_uses_rich_table_when_flag_enabled() {
             };
 
             let (item, has_trailing_newline) =
-                layout_text_block(block, &text_layout, BlockLocation::Middle, false)
+                layout_text_block(block, &text_layout, BlockLocation::Middle, false, false)
                     .expect("table layout should succeed");
 
             assert!(matches!(item, BlockItem::Table(_)));
@@ -726,7 +769,7 @@ fn test_layout_text_block_uses_plain_text_when_flag_disabled() {
             };
 
             let (item, _has_trailing_newline) =
-                layout_text_block(block, &text_layout, BlockLocation::Middle, false)
+                layout_text_block(block, &text_layout, BlockLocation::Middle, false, false)
                     .expect("table layout should succeed");
 
             assert!(matches!(item, BlockItem::Paragraph(_)));

crates/editor/src/render/model/mod.rs

@@ -204,6 +204,7 @@ pub enum HitTestBlockType {
 pub struct RenderLayoutOptions {
     pub render_mermaid_diagrams: bool,
     pub mermaid_render_offsets: HashSet<CharOffset>,
+    pub group_plain_text_lines: bool,
 }
 
 #[derive(Debug)]
@@ -1882,6 +1883,14 @@ impl RenderState {
         true
     }
 
+    pub fn set_group_plain_text_lines(&mut self, group_plain_text_lines: bool) -> bool {
+        if self.layout_options.group_plain_text_lines == group_plain_text_lines {
+            return false;
+        }
+        self.layout_options.group_plain_text_lines = group_plain_text_lines;
+        true
+    }
+
     pub fn set_show_final_trailing_newline_when_non_empty(&mut self, show: bool) {
         if self.show_final_trailing_newline_when_non_empty == show {
             return;
@@ -2612,6 +2621,48 @@ impl RenderState {
         *self.content.borrow_mut() = new_tree;
     }
 
+    fn group_adjacent_plain_text_items(tree: SumTree<BlockItem>) -> SumTree<BlockItem> {
+        fn flush_plain_text(paragraphs: &mut Vec<Paragraph>, tree: &mut SumTree<BlockItem>) {
+            if paragraphs.is_empty() {
+                return;
+            }
+
+            if paragraphs.len() == 1 {
+                let paragraph = paragraphs
+                    .pop()
+                    .expect("paragraphs length was verified to be one");
+                tree.push(BlockItem::Paragraph(paragraph));
+            } else {
+                let paragraphs = Vec1::try_from_vec(mem::take(paragraphs))
+                    .expect("paragraphs length was verified to be greater than one");
+                tree.push(BlockItem::TextBlock {
+                    paragraph_block: ParagraphBlock::new(paragraphs),
+                });
+            }
+        }
+
+        let mut grouped_tree = SumTree::new();
+        let mut pending_plain_text = Vec::new();
+
+        for item in tree.cursor::<(), ()>() {
+            match item {
+                BlockItem::Paragraph(paragraph) => {
+                    pending_plain_text.push(paragraph.clone());
+                }
+                BlockItem::TextBlock { paragraph_block } => {
+                    pending_plain_text.extend(paragraph_block.paragraphs.iter().cloned());
+                }
+                item => {
+                    flush_plain_text(&mut pending_plain_text, &mut grouped_tree);
+                    grouped_tree.push(item.clone());
+                }
+            }
+        }
+
+        flush_plain_text(&mut pending_plain_text, &mut grouped_tree);
+        grouped_tree
+    }
+
     /// Update the render state with laid out new edits.
     fn layout_pending_edit(
         &self,
@@ -2749,6 +2800,9 @@ impl RenderState {
         if let Some(hidden_ranges) = hidden_ranges {
             new_tree = Self::dedupe_hidden_ranges(new_tree, hidden_ranges);
         }
+        if self.layout_options.group_plain_text_lines {
+            new_tree = Self::group_adjacent_plain_text_items(new_tree);
+        }
         log::trace!("Resulting blocks:\n{}", new_tree.describe());
         self.has_final_trailing_newline
             .set(Self::tree_ends_with_trailing_newline(&new_tree));