feat(rome_formatter): Custom separator per Fill item (#3125)

crates/rome_formatter/src/builders.rs

@@ -1653,17 +1653,17 @@ impl<Context> IfGroupBreaks<'_, Context> {
     ///             &format_args![
     ///                 text("["),
     ///                 soft_block_indent(&format_with(|f| {
-    ///                     f.fill(soft_line_break_or_space())
-    ///                         .entry(&text("1,"))
-    ///                         .entry(&text("234568789,"))
-    ///                         .entry(&text("3456789,"))
-    ///                         .entry(&format_args!(
+    ///                     f.fill()
+    ///                         .entry(&soft_line_break_or_space(), &text("1,"))
+    ///                         .entry(&soft_line_break_or_space(), &text("234568789,"))
+    ///                         .entry(&soft_line_break_or_space(), &text("3456789,"))
+    ///                         .entry(&soft_line_break_or_space(), &format_args!(
     ///                             text("["),
     ///                             soft_block_indent(&text("4")),
     ///                             text("]"),
     ///                             if_group_breaks(&text(",")).with_group_id(Some(group_id))
     ///                         ))
-    ///                         .finish()
+    ///                     .finish()
     ///                 })),
     ///                 text("]")
     ///             ],
@@ -2161,40 +2161,26 @@ pub fn get_lines_before<L: Language>(next_node: &SyntaxNode<L>) -> usize {
 pub struct FillBuilder<'fmt, 'buf, Context> {
     result: FormatResult<()>,
     fmt: &'fmt mut Formatter<'buf, Context>,
-
-    /// The separator to use to join the elements
-    separator: FormatElement,
     items: Vec<FormatElement>,
 }
 
 impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> {
-    pub(crate) fn new<Separator>(
-        fmt: &'a mut Formatter<'buf, Context>,
-        separator: Separator,
-    ) -> Self
-    where
-        Separator: Format<Context>,
-    {
-        let mut buffer = VecBuffer::new(fmt.state_mut());
-        let result = write!(buffer, [separator]);
-        let separator = buffer.into_element();
-
+    pub(crate) fn new(fmt: &'a mut Formatter<'buf, Context>) -> Self {
         Self {
-            result,
+            result: Ok(()),
             fmt,
-            separator,
             items: vec![],
         }
     }
 
-    /// Adds an iterator of entries to the fill output.
-    pub fn entries<F, I>(&mut self, entries: I) -> &mut Self
+    /// Adds an iterator of entries to the fill output. Uses the passed `separator` to separate any two items.
+    pub fn entries<F, I>(&mut self, separator: &dyn Format<Context>, entries: I) -> &mut Self
     where
         F: Format<Context>,
         I: IntoIterator<Item = F>,
     {
         for entry in entries {
-            self.entry(&entry);
+            self.entry(separator, &entry);
         }
 
         self
@@ -2207,45 +2193,70 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> {
     ///
     /// The usage of this method is highly discouraged and it's better to use
     /// other APIs on ways: for example progressively format the items based on their type.
-    pub fn flatten_entries<F, I>(&mut self, entries: I) -> &mut Self
+    pub fn flatten_entries<F, I>(
+        &mut self,
+        separator: &dyn Format<Context>,
+        entries: I,
+    ) -> &mut Self
     where
         F: Format<Context>,
         I: IntoIterator<Item = F>,
     {
         for entry in entries {
-            self.flatten_entry(&entry);
+            self.flatten_entry(separator, &entry);
         }
 
         self
     }
 
     /// Adds a new entry to the fill output. If the entry is a [FormatElement::List],
     /// then adds the list's entries to the fill output instead of the list itself.
-    pub fn flatten_entry(&mut self, entry: &dyn Format<Context>) -> &mut Self {
+    fn flatten_entry(
+        &mut self,
+        separator: &dyn Format<Context>,
+        entry: &dyn Format<Context>,
+    ) -> &mut Self {
         self.result = self.result.and_then(|_| {
             let mut buffer = VecBuffer::new(self.fmt.state_mut());
             write!(buffer, [entry])?;
 
-            self.items.extend(buffer.into_vec());
+            let entries = buffer.into_vec();
+            self.items.reserve((entries.len() * 2).saturating_sub(1));
+
+            let mut buffer = VecBuffer::new(self.fmt.state_mut());
+            for item in entries.into_iter() {
+                if !self.items.is_empty() {
+                    write!(buffer, [separator])?;
+
+                    self.items.push(buffer.take_element());
+                }
+
+                self.items.push(item);
+            }
 
             Ok(())
         });
 
         self
     }
 
-    /// Adds a new entry to the fill output.
-    pub fn entry(&mut self, entry: &dyn Format<Context>) -> &mut Self {
+    /// Adds a new entry to the fill output. The `separator` isn't written if this is the first element in the list.
+    pub fn entry(
+        &mut self,
+        separator: &dyn Format<Context>,
+        entry: &dyn Format<Context>,
+    ) -> &mut Self {
         self.result = self.result.and_then(|_| {
             let mut buffer = VecBuffer::new(self.fmt.state_mut());
-            write!(buffer, [entry])?;
-
-            let item = buffer.into_element();
 
-            if !item.is_empty() {
-                self.items.push(item);
+            if !self.items.is_empty() {
+                write!(buffer, [separator])?;
+                self.items.push(buffer.take_element());
             }
 
+            write!(buffer, [entry])?;
+            self.items.push(buffer.into_element());
+
             Ok(())
         });
 
@@ -2260,10 +2271,9 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> {
             match items.len() {
                 0 => Ok(()),
                 1 => self.fmt.write_element(items.pop().unwrap()),
-                _ => self.fmt.write_element(FormatElement::Fill(Fill {
-                    content: items.into_boxed_slice(),
-                    separator: Box::new(self.separator.clone()),
-                })),
+                _ => self
+                    .fmt
+                    .write_element(FormatElement::Fill(items.into_boxed_slice())),
             }
         })
     }

crates/rome_formatter/src/format_element.rs

@@ -66,8 +66,9 @@ pub enum FormatElement {
     List(List),
 
     /// Concatenates multiple elements together with a given separator printed in either
-    /// flat or expanded mode to fill the print width. See [crate::Formatter::fill].
-    Fill(Fill),
+    /// flat or expanded mode to fill the print width. Expect that the content is a list of alternating
+    /// [element, separator] See [crate::Formatter::fill].
+    Fill(Content),
 
     /// A text that should be printed as is, see [crate::text] for documentation and examples.
     Text(Text),
@@ -252,26 +253,6 @@ impl Deref for List {
     }
 }
 
-/// Fill is a list of [FormatElement]s along with a separator.
-///
-/// The printer prints this list delimited by a separator, wrapping the list when it
-/// reaches the specified `line_width`.
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct Fill {
-    pub(super) content: Content,
-    pub(super) separator: Box<FormatElement>,
-}
-
-impl Fill {
-    pub fn content(&self) -> &[FormatElement] {
-        &self.content
-    }
-
-    pub fn separator(&self) -> &FormatElement {
-        &self.separator
-    }
-}
-
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Align {
     pub(super) content: Content,
@@ -688,7 +669,7 @@ impl FormatElement {
             | FormatElement::ConditionalGroupContent(ConditionalGroupContent { content, .. })
             | FormatElement::IndentIfGroupBreaks(IndentIfGroupBreaks { content, .. })
             | FormatElement::Comment(content)
-            | FormatElement::Fill(Fill { content, .. })
+            | FormatElement::Fill(content)
             | FormatElement::Verbatim(Verbatim { content, .. })
             | FormatElement::Label(Label { content, .. })
             | FormatElement::Indent(content)
@@ -976,18 +957,8 @@ impl Format<IrFormatContext> for FormatElement {
                     ]
                 )
             }
-            FormatElement::Fill(fill) => {
-                write!(
-                    f,
-                    [
-                        text("fill("),
-                        fill.separator.as_ref(),
-                        text(","),
-                        space(),
-                        fill.content(),
-                        text(")")
-                    ]
-                )
+            FormatElement::Fill(content) => {
+                write!(f, [text("fill("), content.as_ref(), text(")")])
             }
 
             FormatElement::BestFitting(best_fitting) => {

crates/rome_formatter/src/formatter.rs

@@ -137,11 +137,11 @@ impl<'buf, Context> Formatter<'buf, Context> {
     /// use rome_formatter::{format, format_args};
     ///
     /// let formatted = format!(SimpleFormatContext::default(), [format_with(|f| {
-    ///     f.fill(soft_line_break_or_space())
-    ///         .entry(&text("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
-    ///         .entry(&text("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"))
-    ///         .entry(&text("cccccccccccccccccccccccccccccc"))
-    ///         .entry(&text("dddddddddddddddddddddddddddddd"))
+    ///     f.fill()
+    ///         .entry(&soft_line_break_or_space(), &text("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
+    ///         .entry(&soft_line_break_or_space(), &text("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"))
+    ///         .entry(&soft_line_break_or_space(), &text("cccccccccccccccccccccccccccccc"))
+    ///         .entry(&soft_line_break_or_space(), &text("dddddddddddddddddddddddddddddd"))
     ///         .finish()
     /// })]).unwrap();
     ///
@@ -163,19 +163,16 @@ impl<'buf, Context> Formatter<'buf, Context> {
     /// ];
     ///
     /// let formatted = format!(SimpleFormatContext::default(), [format_with(|f| {
-    ///     f.fill(soft_line_break()).entries(entries.iter()).finish()
+    ///     f.fill().entries(&soft_line_break(), entries.iter()).finish()
     /// })]).unwrap();
     ///
     /// assert_eq!(
     ///     &std::format!("<b>Important: </b>\nPlease do not commit memory bugs such as segfaults, buffer overflows, etc. otherwise you \n<em>will</em> be reprimanded"),
     ///     formatted.print().as_code()
     /// )
     /// ```
-    pub fn fill<'a, Separator>(&'a mut self, separator: Separator) -> FillBuilder<'a, 'buf, Context>
-    where
-        Separator: Format<Context>,
-    {
-        FillBuilder::new(self, separator)
+    pub fn fill<'a>(&'a mut self) -> FillBuilder<'a, 'buf, Context> {
+        FillBuilder::new(self)
     }
 
     /// Formats `content` into an interned element without writing it to the formatter's buffer.