diff --git a/crates/rome_formatter/src/builders.rs b/crates/rome_formatter/src/builders.rs index b7d81d34c5b..5ca19339d4f 100644 --- a/crates/rome_formatter/src/builders.rs +++ b/crates/rome_formatter/src/builders.rs @@ -1653,17 +1653,17 @@ impl 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("]") /// ], @@ -2041,40 +2041,26 @@ pub fn get_lines_before(next_node: &SyntaxNode) -> 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, } impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { - pub(crate) fn new( - fmt: &'a mut Formatter<'buf, Context>, - separator: Separator, - ) -> Self - where - Separator: Format, - { - 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(&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(&mut self, separator: &dyn Format, entries: I) -> &mut Self where F: Format, I: IntoIterator, { for entry in entries { - self.entry(&entry); + self.entry(separator, &entry); } self @@ -2087,13 +2073,17 @@ 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(&mut self, entries: I) -> &mut Self + pub fn flatten_entries( + &mut self, + separator: &dyn Format, + entries: I, + ) -> &mut Self where F: Format, I: IntoIterator, { for entry in entries { - self.flatten_entry(&entry); + self.flatten_entry(separator, &entry); } self @@ -2101,12 +2091,28 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { /// 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) -> &mut Self { + fn flatten_entry( + &mut self, + separator: &dyn Format, + entry: &dyn Format, + ) -> &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(()) }); @@ -2114,18 +2120,23 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { self } - /// Adds a new entry to the fill output. - pub fn entry(&mut self, entry: &dyn Format) -> &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, + entry: &dyn Format, + ) -> &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(()) }); @@ -2140,10 +2151,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())), } }) } diff --git a/crates/rome_formatter/src/format_element.rs b/crates/rome_formatter/src/format_element.rs index 2471c3f6d3b..01bd6cb0f0a 100644 --- a/crates/rome_formatter/src/format_element.rs +++ b/crates/rome_formatter/src/format_element.rs @@ -62,8 +62,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), @@ -247,26 +248,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, -} - -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, @@ -669,7 +650,7 @@ impl FormatElement { FormatElement::Group(Group { content, .. }) | FormatElement::ConditionalGroupContent(ConditionalGroupContent { content, .. }) | FormatElement::Comment(content) - | FormatElement::Fill(Fill { content, .. }) + | FormatElement::Fill(content) | FormatElement::Verbatim(Verbatim { content, .. }) | FormatElement::Label(Label { content, .. }) | FormatElement::Indent(content) @@ -933,18 +914,8 @@ impl Format 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) => { diff --git a/crates/rome_formatter/src/formatter.rs b/crates/rome_formatter/src/formatter.rs index 8d447d2309b..b522637e2a1 100644 --- a/crates/rome_formatter/src/formatter.rs +++ b/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,7 +163,7 @@ 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!( @@ -171,11 +171,8 @@ impl<'buf, Context> Formatter<'buf, Context> { /// formatted.print().as_code() /// ) /// ``` - pub fn fill<'a, Separator>(&'a mut self, separator: Separator) -> FillBuilder<'a, 'buf, Context> - where - Separator: Format, - { - 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. diff --git a/crates/rome_formatter/src/printer/mod.rs b/crates/rome_formatter/src/printer/mod.rs index 091c76209d2..cba58a040e7 100644 --- a/crates/rome_formatter/src/printer/mod.rs +++ b/crates/rome_formatter/src/printer/mod.rs @@ -5,7 +5,6 @@ pub use printer_options::*; use crate::format_element::{ Align, ConditionalGroupContent, DedentMode, Group, LineMode, PrintMode, VerbatimKind, }; -use crate::intersperse::Intersperse; use crate::{FormatElement, GroupId, IndentStyle, Printed, SourceMarker, TextRange}; use rome_rowan::{TextLen, TextSize}; @@ -170,8 +169,8 @@ impl<'a> Printer<'a> { } } - FormatElement::Fill(fill) => { - self.print_fill(queue, fill.content(), fill.separator(), args); + FormatElement::Fill(content) => { + self.print_fill(queue, content, args); } FormatElement::List(list) => { @@ -378,7 +377,6 @@ impl<'a> Printer<'a> { &mut self, queue: &mut ElementCallQueue<'a>, content: &'a [FormatElement], - separator: &'a FormatElement, args: PrintElementArgs, ) { let empty_rest = ElementCallQueue::default(); @@ -411,50 +409,64 @@ impl<'a> Printer<'a> { ); // Process remaining items - for next_item in items { - // A line break in expanded mode is always necessary if the current item didn't fit. - // otherwise see if both contents fit on the line. - let current_and_next_fit = current_fits - && fits_on_line( - [separator, next_item], - args.with_print_mode(PrintMode::Flat), - &empty_rest, - self, - ); - - if current_and_next_fit { - // Print Space and next item on the same line - self.print_all( - queue, - &[separator, next_item], - args.with_print_mode(PrintMode::Flat), - ); - } else { - // Print the separator and then check again if the next item fits on the line now - self.print_all( - queue, - &[separator], - args.with_print_mode(PrintMode::Expanded), - ); - - let next_fits = fits_on_line( - [next_item], - args.with_print_mode(PrintMode::Flat), - &empty_rest, - self, - ); + loop { + match (items.next(), items.next()) { + (Some(separator), Some(next_item)) => { + // A line break in expanded mode is always necessary if the current item didn't fit. + // otherwise see if both contents fit on the line. + let current_and_next_fit = current_fits + && fits_on_line( + [separator, next_item], + args.with_print_mode(PrintMode::Flat), + &empty_rest, + self, + ); + + if current_and_next_fit { + // Print Space and next item on the same line + self.print_all( + queue, + &[separator, next_item], + args.with_print_mode(PrintMode::Flat), + ); + } else { + // Print the separator and then check again if the next item fits on the line now + self.print_all( + queue, + &[separator], + args.with_print_mode(PrintMode::Expanded), + ); + + let next_fits = fits_on_line( + [next_item], + args.with_print_mode(PrintMode::Flat), + &empty_rest, + self, + ); + + if next_fits { + self.print_all( + queue, + &[next_item], + args.with_print_mode(PrintMode::Flat), + ); + } else { + self.print_all( + queue, + &[next_item], + args.with_print_mode(PrintMode::Expanded), + ); + } - if next_fits { - self.print_all(queue, &[next_item], args.with_print_mode(PrintMode::Flat)); - } else { - self.print_all( - queue, - &[next_item], - args.with_print_mode(PrintMode::Expanded), - ); + current_fits = next_fits; + } + } + (Some(_), _) => { + // Don't print a trailing separator + } + (None, _) => { + break; } - - current_fits = next_fits; } } } @@ -924,10 +936,10 @@ fn fits_element_on_line<'a, 'rest>( FormatElement::List(list) => queue.extend(list.iter(), args), - FormatElement::Fill(fill) => queue.queue.0.extend( - Intersperse::new(fill.content().iter().rev(), fill.separator()) - .map(|t| PrintElementCall::new(t, args)), - ), + FormatElement::Fill(content) => queue + .queue + .0 + .extend(content.iter().rev().map(|t| PrintElementCall::new(t, args))), FormatElement::Text(token) => { let indent = std::mem::take(&mut state.pending_indent); @@ -1109,7 +1121,7 @@ mod tests { write!(&mut buffer, [root]).unwrap(); - Printer::new(options).print(&buffer.into_element()) + Printer::new(options).print(&dbg!(buffer.into_element())) } #[test] @@ -1301,25 +1313,43 @@ two lines`, let mut formatter = Formatter::new(&mut buffer); formatter - .fill(&soft_line_break_or_space()) + .fill() // These all fit on the same line together - .entry(&format_args!(text("1"), text(","))) - .entry(&format_args!(text("2"), text(","))) - .entry(&format_args!(text("3"), text(","))) + .entry( + &soft_line_break_or_space(), + &format_args!(text("1"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("2"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("3"), text(",")), + ) // This one fits on a line by itself, - .entry(&format_args!(text("723493294"), text(","))) + .entry( + &soft_line_break_or_space(), + &format_args!(text("723493294"), text(",")), + ) // fits without breaking - .entry(&group(&format_args!( - text("["), - soft_block_indent(&text("5")), - text("],") - ))) + .entry( + &soft_line_break_or_space(), + &group(&format_args!( + text("["), + soft_block_indent(&text("5")), + text("],") + )), + ) // this one must be printed in expanded mode to fit - .entry(&group(&format_args!( - text("["), - soft_block_indent(&text("123456789")), - text("]"), - ))) + .entry( + &soft_line_break_or_space(), + &group(&format_args!( + text("["), + soft_block_indent(&text("123456789")), + text("]"), + )), + ) .finish() .unwrap(); @@ -1340,10 +1370,19 @@ two lines`, group(&format_args![ text("["), soft_block_indent(&format_with(|f| { - f.fill(soft_line_break_or_space()) - .entry(&format_args!(text("1"), text(","))) - .entry(&format_args!(text("2"), text(","))) - .entry(&format_args!(text("3"), if_group_breaks(&text(",")))) + f.fill() + .entry( + &soft_line_break_or_space(), + &format_args!(text("1"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("2"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("3"), if_group_breaks(&text(","))), + ) .finish() })), text("]") diff --git a/crates/rome_js_formatter/src/js/lists/array_element_list.rs b/crates/rome_js_formatter/src/js/lists/array_element_list.rs index d29f19de3e3..ce1463d3254 100644 --- a/crates/rome_js_formatter/src/js/lists/array_element_list.rs +++ b/crates/rome_js_formatter/src/js/lists/array_element_list.rs @@ -29,8 +29,9 @@ impl FormatRule for FormatJsArrayElementList { if !has_formatter_trivia(node.syntax()) && can_print_fill(node) { // Using format_separated is valid in this case as can_print_fill does not allow holes return f - .fill(soft_line_break_or_space()) + .fill() .entries( + &soft_line_break_or_space(), node.format_separated(JsSyntaxKind::COMMA) .with_group_id(self.group_id), ) diff --git a/crates/rome_js_formatter/src/jsx/lists/child_list.rs b/crates/rome_js_formatter/src/jsx/lists/child_list.rs index 195ae7d9645..7a2377337ca 100644 --- a/crates/rome_js_formatter/src/jsx/lists/child_list.rs +++ b/crates/rome_js_formatter/src/jsx/lists/child_list.rs @@ -12,8 +12,8 @@ impl FormatRule for FormatJsxChildList { fn fmt(&self, node: &JsxChildList, formatter: &mut JsFormatter) -> FormatResult<()> { if contains_meaningful_jsx_text(node) { formatter - .fill(soft_line_break()) - .flatten_entries(node.iter().formatted()) + .fill() + .flatten_entries(&soft_line_break(), node.iter().formatted()) .finish() } else { formatter