Skip to content

Commit

Permalink
First kind of working solution
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Oct 15, 2024
1 parent 22cb7d4 commit 0a86ffe
Show file tree
Hide file tree
Showing 9 changed files with 217 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ impl NeedsParentheses for ExprStringLiteral {
_parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
// TODO: This is complicated. We should only return multiline if we *know* that
// it won't fit because of how assignment works.
if self.value.is_implicit_concatenated() {
OptionalParentheses::Multiline
} else if AnyString::String(self).is_multiline(context.source()) {
Expand Down
18 changes: 9 additions & 9 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ mod tests {
use ruff_python_trivia::CommentRanges;
use ruff_text_size::{TextRange, TextSize};

use crate::{format_module_ast, format_module_source, format_range, PyFormatOptions};
use crate::{
format_module_ast, format_module_source, format_range, PreviewMode, PyFormatOptions,
};

/// Very basic test intentionally kept very similar to the CLI
#[test]
Expand Down Expand Up @@ -188,13 +190,10 @@ if True:
#[test]
fn quick_test() {
let source = r#"
def main() -> None:
if True:
some_very_long_variable_name_abcdefghijk = Foo()
some_very_long_variable_name_abcdefghijk = some_very_long_variable_name_abcdefghijk[
some_very_long_variable_name_abcdefghijk.some_very_long_attribute_name
== "This is a very long string abcdefghijk"
]
(
f'{one}'
f'{two}'
)
"#;
let source_type = PySourceType::Python;
Expand All @@ -203,7 +202,8 @@ def main() -> None:
let source_path = "code_inline.py";
let parsed = parse(source, source_type.as_mode()).unwrap();
let comment_ranges = CommentRanges::from(parsed.tokens());
let options = PyFormatOptions::from_extension(Path::new(source_path));
let options = PyFormatOptions::from_extension(Path::new(source_path))
.with_preview(PreviewMode::Enabled);
let formatted = format_module_ast(&parsed, &comment_ranges, source, options).unwrap();

// Uncomment the `dbg` to print the IR.
Expand Down
11 changes: 3 additions & 8 deletions crates/ruff_python_formatter/src/other/f_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,9 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
let quotes = StringQuotes::from(string_kind);
write!(f, [string_kind.prefix(), quotes])?;

f.join()
.entries(
self.value
.elements
.iter()
.map(|element| FormatFStringElement::new(element, context)),
)
.finish()?;
for element in &self.value.elements {
FormatFStringElement::new(element, context).fmt(f)?;
}

// Ending quote
quotes.fmt(f)
Expand Down
8 changes: 3 additions & 5 deletions crates/ruff_python_formatter/src/other/f_string_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,9 @@ impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
if let Some(format_spec) = format_spec.as_deref() {
token(":").fmt(f)?;

f.join()
.entries(format_spec.elements.iter().map(|element| {
FormatFStringElement::new(element, self.context.f_string())
}))
.finish()?;
for element in &format_spec.elements {
FormatFStringElement::new(element, self.context.f_string()).fmt(f)?;
}

// These trailing comments can only occur if the format specifier is
// present. For example,
Expand Down
13 changes: 6 additions & 7 deletions crates/ruff_python_formatter/src/string/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use memchr::memchr2;

use ruff_python_ast::{
self as ast, AnyNodeRef, AnyStringFlags, Expr, ExprBytesLiteral, ExprFString,
ExprStringLiteral, ExpressionRef, StringFlags, StringLiteral,
ExprStringLiteral, ExpressionRef, FStringElement, FStringPart, StringFlags, StringLiteral,
};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};
Expand Down Expand Up @@ -68,12 +68,11 @@ impl<'a> AnyString<'a> {

pub(crate) fn is_multiline(self, source: &str) -> bool {
match self {
AnyString::String(_) | AnyString::Bytes(_) => {
self.parts()
.next()
.is_some_and(|part| part.flags().is_triple_quoted())
&& memchr2(b'\n', b'\r', source[self.range()].as_bytes()).is_some()
}
AnyString::String(_) | AnyString::Bytes(_) => self.parts().any(|part| {
part.flags().is_triple_quoted()
&& memchr2(b'\n', b'\r', source[part.range()].as_bytes()).is_some()
}),
// TODO: Should this only test string literals and expressions (if they contain any newline)?
AnyString::FString(fstring) => {
memchr2(b'\n', b'\r', source[fstring.range].as_bytes()).is_some()
}
Expand Down
73 changes: 56 additions & 17 deletions crates/ruff_python_formatter/src/string/mod.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
pub(crate) use any::AnyString;
pub(crate) use normalize::{normalize_string, NormalizedString, StringNormalizer};
use ruff_formatter::format_args;
use ruff_formatter::{format_args, write};
use ruff_python_ast::str::Quote;
use ruff_python_ast::{
str_prefix::{AnyStringPrefix, StringLiteralPrefix},
AnyStringFlags, StringFlags,
};
use ruff_text_size::Ranged;
use std::borrow::Cow;

use crate::comments::{leading_comments, trailing_comments};
use crate::expression::parentheses::in_parentheses_only_soft_line_break_or_space;
use crate::other::f_string::FormatFString;
use crate::other::string_literal::StringLiteralKind;
use crate::prelude::*;
use crate::preview::is_f_string_formatting_enabled;
use crate::string::any::AnyStringPart;
use crate::string::normalize::QuoteMetadata;
use crate::QuoteStyle;
Expand Down Expand Up @@ -69,7 +70,7 @@ impl<'a> FormatImplicitConcatenatedString<'a> {

// Don't merge multiline strings because that's pointless, a multiline string can
// never fit on a single line.
if self.string.is_multiline(context.source()) {
if !self.string.is_fstring() && self.string.is_multiline(context.source()) {
return None;
}

Expand All @@ -81,14 +82,17 @@ impl<'a> FormatImplicitConcatenatedString<'a> {

// TODO unify quote styles.
// Possibly run directly on entire string?
let first_part = self.string.parts().next()?;

for part in self.string.parts() {
let Ok(preferred_quote) = Quote::try_from(normalizer.preferred_quote_style(part))
else {
// TOOD: Handle preserve in some way or another
return None;
};
// Only determining the preferred quote for the first string is sufficient
// because we don't support joining triple quoted strings with non triple quoted strings.
let Ok(preferred_quote) = Quote::try_from(normalizer.preferred_quote_style(first_part))
else {
// TODO: Handle preserve
return None;
};

for part in self.string.parts() {
// Again, this takes a StringPart and not a `AnyStringPart`.
let part_quote_metadata = QuoteMetadata::from_part(part, preferred_quote, context);

Expand All @@ -100,7 +104,7 @@ impl<'a> FormatImplicitConcatenatedString<'a> {
}
}

Some(merged_quotes?.choose(Quote::Double))
Some(merged_quotes?.choose(preferred_quote))
}
}

Expand All @@ -109,12 +113,6 @@ impl Format<PyFormatContext<'_>> for FormatImplicitConcatenatedString<'_> {
let comments = f.context().comments().clone();
let quoting = self.string.quoting(f.context().locator());

// let cant_collapse = self.string.is_multiline(f.context().source())
// || self.string.parts().any(|part| {
// let part_comments = comments.leading_dangling_trailing(&part);
// part_comments.has_leading() || part_comments.has_trailing()
// });

let format_expanded = format_with(|f| {
let mut joiner = f.join_with(in_parentheses_only_soft_line_break_or_space());
for part in self.string.parts() {
Expand Down Expand Up @@ -145,7 +143,48 @@ impl Format<PyFormatContext<'_>> for FormatImplicitConcatenatedString<'_> {
joiner.finish()
});

format_expanded.fmt(f)
if let Some(collapsed_quotes) = dbg!(self.merged_prefix(f.context())) {
let format_flat = format_with(|f| {
let mut parts = self.string.parts();

let Some(first_part) = parts.next() else {
return Ok(());
};

// TODO handle different prefixes, e.g. f-string and non fstrings. We should probably handle this
// inside of `merged_prefix`
let flags = first_part.flags().with_quote_style(collapsed_quotes);
let quotes = StringQuotes::from(flags);

write!(f, [flags.prefix(), quotes])?;

for part in self.string.parts() {
let content = f.context().locator().slice(part.content_range());
let normalized = normalize_string(
content,
0,
flags,
is_f_string_formatting_enabled(f.context()),
);
match normalized {
Cow::Borrowed(_) => source_text_slice(part.content_range()).fmt(f)?,
Cow::Owned(normalized) => text(&normalized).fmt(f)?,
}
}

quotes.fmt(f)
});

write!(
f,
[
if_group_fits_on_line(&format_flat),
if_group_breaks(&format_expanded)
]
)
} else {
format_expanded.fmt(f)
}
}
}

Expand Down
Loading

0 comments on commit 0a86ffe

Please sign in to comment.