Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
address code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Aug 16, 2022
1 parent 6480ecf commit 8c57ad3
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 17 deletions.
4 changes: 3 additions & 1 deletion crates/rome_formatter/src/printer/printer_options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ impl PrintWidth {
Self(width)
}

/// Creates a print width that avoids ever breaking content because it exceeds the print width.
/// Returns a print width that guarantees that any content, regardless of its width, fits on the line.
///
/// This has the effect that the printer never prints a line break for any soft line break.
pub fn infinite() -> Self {
Self(u32::MAX)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,32 @@ fn template_literal_contains_new_line(template: &JsTemplate) -> bool {
})
}

/// Returns `true` for a template that starts on the same line as the previous token and contains a line break.
///
///
/// # Examples
//
/// ```javascript
/// "test" + `
/// some content
/// `;
/// ```
///
/// Returns `true` because the template starts on the same line as the `+` token and its text contains a line break.
///
/// ```javascript
/// "test" + `no line break`
/// ```
///
/// Returns `false` because the template text contains no line break.
///
/// ```javascript
/// "test" +
/// `template
/// with line break`;
/// ```
///
/// Returns `false` because the template isn't on the same line as the '+' token.
fn is_multiline_template_starting_on_same_line(template: &JsTemplate) -> bool {
let contains_new_line = template_literal_contains_new_line(template);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@ impl AnyTemplateChunkElement {

impl Format<JsFormatContext> for AnyTemplateChunkElement {
fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
// Per https://tc39.es/ecma262/multipage/ecmascript-language-lexical-grammar.html#sec-static-semantics-trv:
// In template literals, the '\r' and '\r\n' line terminators are normalized to '\n'

let chunk = self.template_chunk_token()?;

write!(
f,
[format_replaced(
&chunk,
&syntax_token_cow_slice(
// Per https://tc39.es/ecma262/multipage/ecmascript-language-lexical-grammar.html#sec-static-semantics-trv:
// In template literals, the '\r' and '\r\n' line terminators are normalized to '\n'
normalize_newlines(chunk.text_trimmed(), ['\r']),
&chunk,
chunk.text_trimmed_range().start(),
Expand Down
22 changes: 16 additions & 6 deletions crates/rome_js_formatter/src/js/expressions/template_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ declare_node_union! {
pub struct TemplateElementOptions {
pub(crate) layout: TemplateElementLayout,

// The indention to use for this element
/// The indention to use for this element
pub(crate) indention: TemplateElementIndention,

// Does the last template chunk (text element) end with a new line?
/// Does the last template chunk (text element) end with a new line?
pub(crate) after_new_line: bool,
}

Expand Down Expand Up @@ -75,7 +75,16 @@ impl Format<JsFormatContext> for FormatTemplateElement {
});

let format_inner = format_with(|f: &mut JsFormatter| match self.options.layout {
TemplateElementLayout::Simple => {
TemplateElementLayout::SingleLine => {
// The goal is to print the expression on a single line, even if it exceeds the configured print width.
//
// Ideally, it would be possible to use a custom buffer that drops all soft line breaks
// (or converts them to spaces). However, this isn't straightforward with our
// nested IR (but would be with a flat ir).
//
// That's why we write the expression into a temporary buffer and print it
// with a printer that uses a print width so large, that the expression never exceeds
// the print width.
let mut buffer = VecBuffer::new(f.state_mut());
write!(buffer, [format_expression])?;
let root = buffer.into_element();
Expand All @@ -94,7 +103,7 @@ impl Format<JsFormatContext> for FormatTemplateElement {
)]
)
}
TemplateElementLayout::Normal => {
TemplateElementLayout::Fit => {
use JsAnyExpression::*;

let expression = self.element.expression();
Expand Down Expand Up @@ -184,6 +193,7 @@ impl AnyTemplateElement {
}
}

/// Writes `content` with the specified `indention`.
fn write_with_indention<Content>(
content: &Content,
indention: TemplateElementIndention,
Expand All @@ -200,8 +210,8 @@ where
return write!(f, [content]);
}

// Adds as many nested `indent` elements until it reaches the desired indent level.
let format_indented = format_with(|f| {
// Nest indents to get to the same indention level
if level == 0 {
write!(f, [content])
} else {
Expand All @@ -219,8 +229,8 @@ where
}
});

// Adds any necessary `align` for spaces not covered by indent level.
let format_aligned = format_with(|f| {
// Add any potential spaces in the end
if spaces == 0 {
write!(f, [format_indented])
} else {
Expand Down
21 changes: 14 additions & 7 deletions crates/rome_js_formatter/src/js/lists/template_element_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ pub(crate) enum AnyTemplateElementList {
impl Format<JsFormatContext> for AnyTemplateElementList {
fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
let layout = if self.is_simple() {
TemplateElementLayout::Simple
TemplateElementLayout::SingleLine
} else {
TemplateElementLayout::Normal
TemplateElementLayout::Fit
};

let mut indention = TemplateElementIndention::default();
Expand Down Expand Up @@ -82,6 +82,11 @@ impl Format<JsFormatContext> for AnyTemplateElementList {
}

impl AnyTemplateElementList {
/// Returns `true` for `JsTemplate` if all elements are simple expressions that should be printed on a single line.
///
/// Simple expressions are:
/// * Identifiers: `this`, `a`
/// * Members: `a.b`, `a[b]`, `a.b[c].d`, `a.b[5]`, `a.b["test"]`
fn is_simple(&self) -> bool {
match self {
AnyTemplateElementList::JsTemplateElementList(list) => {
Expand Down Expand Up @@ -119,15 +124,17 @@ impl AnyTemplateElementList {

#[derive(Debug, Copy, Clone)]
pub enum TemplateElementLayout {
/// Applied when all expressions are identifier, `this`, static member expressions, or computed member expressions with number or string literals.
/// Formats every expression without any line breaks.
Simple,
Normal,
/// Applied when all expressions are identifiers, `this`, static member expressions, or computed member expressions with number or string literals.
/// Formats the expressions on a single line, even if their width otherwise would exceed the print width.
SingleLine,

/// Tries to format the expression on a single line but may break the expression if the line otherwise exceeds the print width.
Fit,
}

impl Default for TemplateElementLayout {
fn default() -> Self {
TemplateElementLayout::Normal
TemplateElementLayout::Fit
}
}

Expand Down

0 comments on commit 8c57ad3

Please sign in to comment.