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

Preferred string quotes #2443

Closed
Tracked by #2403
MichaReiser opened this issue Apr 14, 2022 · 4 comments · Fixed by #2624
Closed
Tracked by #2403

Preferred string quotes #2443

MichaReiser opened this issue Apr 14, 2022 · 4 comments · Fixed by #2624
Assignees
Labels
A-Formatter Area: formatter I-Normal Implementation: normal understanding of the tool and awareness

Comments

@MichaReiser
Copy link
Contributor

MichaReiser commented Apr 14, 2022

Prettier detects the ideal quote to use for a string so that the least amount of escaped quotes are necessary:

function getPreferredQuote(rawContent, preferredQuote) {
  /** @type {{ quote: '"', regex: RegExp, escaped: """ }} */
  const double = { quote: '"', regex: /"/g, escaped: """ };
  /** @type {{ quote: "'", regex: RegExp, escaped: "'" }} */
  const single = { quote: "'", regex: /'/g, escaped: "'" };

  const preferred = preferredQuote === "'" ? single : double;
  const alternate = preferred === single ? double : single;

  let result = preferred;

  // If `rawContent` contains at least one of the quote preferred for enclosing
  // the string, we might want to enclose with the alternate quote instead, to
  // minimize the number of escaped quotes.
  if (
    rawContent.includes(preferred.quote) ||
    rawContent.includes(alternate.quote)
  ) {
    const numPreferredQuotes = (rawContent.match(preferred.regex) || []).length;
    const numAlternateQuotes = (rawContent.match(alternate.regex) || []).length;

    result = numPreferredQuotes > numAlternateQuotes ? alternate : preferred;
  }

  return result;
}

Playground

Rome should do the same when printing string literal tokens.

Input

 // Single double quote.
('"');
('"');
 
 // Single single quote.
("'");
("'");

// One of each
"\"'";
'"\'';

// One of each with unnecessary escapes.
("\"'");
("\"'");
 
 // More double quotes than single quotes.
('"\'"');
('"\'"');
 
 // More single quotes than double quotes.
("\"''");
("\"''");
 
 // Two of each.
("\"\"''");
("\"\"''");


("He's sayin': \"How's it goin'?\" Don't ask me why.");
("He's sayin': \"How's it goin'?\" Don't ask me why.");
 
 // Somewhat more real-word example 2.
('var backslash = "\\", doubleQuote = \'"\';');
('var backslash = "\\", doubleQuote = \'"\';');

Prettier

// Single double quote.
('"');
('"');

// Single single quote.
("'");
("'");

// One of each
("\"'");
("\"'");

// One of each with unnecessary escapes.
("\"'");
("\"'");

// More double quotes than single quotes.
('"\'"');
('"\'"');

// More single quotes than double quotes.
("\"''");
("\"''");

// Two of each.
("\"\"''");
("\"\"''");

("He's sayin': \"How's it goin'?\" Don't ask me why.");
("He's sayin': \"How's it goin'?\" Don't ask me why.");

// Somewhat more real-word example 2.
('var backslash = "\\", doubleQuote = \'"\';');
('var backslash = "\\", doubleQuote = \'"\';');

Rome

// Single double quote.
('"');
('"');

// Single single quote.
("'");
("'");

// One of each
"\"'";
'"\'';

// One of each with unnecessary escapes.
("\"'");
("\"'");

// More double quotes than single quotes.
('"\'"');
('"\'"');

// More single quotes than double quotes.
("\"''");
("\"''");

// Two of each.
("\"\"''");
("\"\"''");

("He's sayin': \"How's it goin'?\" Don't ask me why.");
("He's sayin': \"How's it goin'?\" Don't ask me why.");

// Somewhat more real-word example 2.
('var backslash = "\\", doubleQuote = \'"\';');
('var backslash = "\\", doubleQuote = \'"\';');

Expected

Rome's formatting to match prettier's.

@MichaReiser MichaReiser added the A-Formatter Area: formatter label Apr 14, 2022
@ematipico ematipico added the good first issue Good for newcomers label Apr 14, 2022
@ktfth
Copy link
Contributor

ktfth commented Apr 22, 2022

Learning the new code base of Rome, someone can point out a place to begin with that contribution?

@MichaReiser
Copy link
Contributor Author

MichaReiser commented Apr 22, 2022

Learning the new code base of Rome, someone can point out a place to begin with that contribution?

pub(crate) fn format_string_literal_token(
token: JsSyntaxToken,
formatter: &Formatter,
) -> FormatElement {
let quoted = token.text_trimmed();
let (primary_quote_char, secondary_quote_char) = match formatter.options().quote_style {
QuoteStyle::Double => ('"', '\''),
QuoteStyle::Single => ('\'', '"'),
};
let content =
if quoted.starts_with(secondary_quote_char) && !quoted.contains(primary_quote_char) {
let s = &quoted[1..quoted.len() - 1];
let s = format!("{}{}{}", primary_quote_char, s, primary_quote_char);
Cow::Owned(normalize_newlines(&s, ['\r']).into_owned())
} else {
normalize_newlines(quoted, ['\r'])
};
formatter.format_replaced(
&token,
Token::from_syntax_token_cow_slice(content, &token, token.text_trimmed_range().start())
.into(),
)
}

This part is responsible for formatting string literals but currently always picks the quote-style specified in the options.

@ematipico ematipico added the I-Normal Implementation: normal understanding of the tool and awareness label May 5, 2022
@ematipico
Copy link
Contributor

I will look into this

@ematipico
Copy link
Contributor

Removing the "good first issue" because it's a sensitive part of the formatter (string manipulation)

@ematipico ematipico moved this to Done in Rome 2022 May 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter I-Normal Implementation: normal understanding of the tool and awareness
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants