Skip to content

Commit

Permalink
Allow parenthesized content exceed configured line width (#7490)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Sep 20, 2023
1 parent 5849a75 commit 192463c
Show file tree
Hide file tree
Showing 13 changed files with 171 additions and 151 deletions.
17 changes: 10 additions & 7 deletions crates/ruff_formatter/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2155,15 +2155,18 @@ impl<Context> std::fmt::Debug for IndentIfGroupBreaks<'_, Context> {
}
}

/// Changes the definition of *fits* for `content`. Instead of measuring it in *flat*, measure it with
/// all line breaks expanded and test if no line exceeds the line width. The [`FitsExpanded`] acts
/// as a expands boundary similar to best fitting, meaning that a [`hard_line_break`] will not cause the parent group to expand.
/// Changes the definition of *fits* for `content`. It measures the width of all lines and allows
/// the content inside of the [`fits_expanded`] to exceed the configured line width. The content
/// coming before and after [`fits_expanded`] must fit into the configured line width.
///
/// The [`fits_expanded`] acts as a expands boundary similar to best fitting,
/// meaning that a [`hard_line_break`] will not cause the parent group to expand.
///
/// Useful in conjunction with a group with a condition.
///
/// ## Examples
/// The outer group with the binary expression remains *flat* regardless of the array expression
/// that spans multiple lines.
/// The outer group with the binary expression remains *flat* regardless of the array expression that
/// spans multiple lines with items exceeding the configured line width.
///
/// ```
/// # use ruff_formatter::{format, format_args, LineWidth, SimpleFormatOptions, write};
Expand All @@ -2183,7 +2186,7 @@ impl<Context> std::fmt::Debug for IndentIfGroupBreaks<'_, Context> {
/// token("["),
/// soft_block_indent(&format_args![
/// token("a,"), space(), token("# comment"), expand_parent(), soft_line_break_or_space(),
/// token("b")
/// token("'A very long string that exceeds the configured line width of 80 characters but the enclosing binary expression still fits.'")
/// ]),
/// token("]")
/// ]))
Expand All @@ -2194,7 +2197,7 @@ impl<Context> std::fmt::Debug for IndentIfGroupBreaks<'_, Context> {
/// let formatted = format!(SimpleFormatContext::default(), [content])?;
///
/// assert_eq!(
/// "a + [\n\ta, # comment\n\tb\n]",
/// "a + [\n\ta, # comment\n\t'A very long string that exceeds the configured line width of 80 characters but the enclosing binary expression still fits.'\n]",
/// formatted.print()?.as_code()
/// );
/// # Ok(())
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff_formatter/src/format_element/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,14 @@ pub enum Tag {
StartFitsExpanded(FitsExpanded),
EndFitsExpanded,

/// Marks the start and end of a best-fitting variant.
StartBestFittingEntry,
EndBestFittingEntry,

/// Parenthesizes the content but only if adding the parentheses and indenting the content
/// makes the content fit in the configured line width.
///
/// See [`crate::builders::best_fit_parenthesize`] for an in-depth explanation.
StartBestFitParenthesize {
id: Option<GroupId>,
},
Expand Down
2 changes: 0 additions & 2 deletions crates/ruff_formatter/src/format_extensions.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(dead_code)]

use crate::prelude::*;
use std::cell::OnceCell;
use std::marker::PhantomData;
Expand Down
82 changes: 54 additions & 28 deletions crates/ruff_formatter/src/printer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
// line break should be printed as regular line break
return Ok(Fits::Yes);
}
MeasureMode::AllLines => {
MeasureMode::AllLines | MeasureMode::AllLinesAllowTextOverflow => {
// Continue measuring on the next line
self.state.line_width = 0;
self.state.pending_indent = args.indention();
Expand Down Expand Up @@ -1354,9 +1354,11 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}

FormatElement::Tag(StartLineSuffix { reserved_width }) => {
self.state.line_width += reserved_width;
if self.state.line_width > self.options().line_width.into() {
return Ok(Fits::No);
if *reserved_width > 0 {
self.state.line_width += reserved_width;
if self.state.line_width > self.options().line_width.into() {
return Ok(Fits::No);
}
}
self.queue.skip_content(TagKind::LineSuffix);
self.state.has_line_suffix = true;
Expand All @@ -1370,32 +1372,42 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
condition,
propagate_expand,
})) => {
let condition_met = match condition {
Some(condition) => {
let group_mode = match condition.group_id {
Some(group_id) => self.group_modes().get_print_mode(group_id)?,
None => args.mode(),
match args.mode() {
PrintMode::Expanded => {
// As usual, nothing to measure
self.stack.push(TagKind::FitsExpanded, args);
}
PrintMode::Flat => {
let condition_met = match condition {
Some(condition) => {
let group_mode = match condition.group_id {
Some(group_id) => {
self.group_modes().get_print_mode(group_id)?
}
None => args.mode(),
};

condition.mode == group_mode
}
None => true,
};

condition.mode == group_mode
}
None => true,
};
if condition_met {
// Measure in fully expanded mode and allow overflows
self.stack.push(
TagKind::FitsExpanded,
args.with_measure_mode(MeasureMode::AllLinesAllowTextOverflow)
.with_print_mode(PrintMode::Expanded),
);
} else {
if propagate_expand.get() {
return Ok(Fits::No);
}

if condition_met {
// Measure in fully expanded mode.
self.stack.push(
TagKind::FitsExpanded,
args.with_print_mode(PrintMode::Expanded)
.with_measure_mode(MeasureMode::AllLines),
);
} else {
if propagate_expand.get() && args.mode().is_flat() {
return Ok(Fits::No);
// As usual
self.stack.push(TagKind::FitsExpanded, args);
}
}

// As usual
self.stack.push(TagKind::FitsExpanded, args);
}
}

Expand Down Expand Up @@ -1482,7 +1494,8 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}
match args.measure_mode() {
MeasureMode::FirstLine => return Fits::Yes,
MeasureMode::AllLines => {
MeasureMode::AllLines
| MeasureMode::AllLinesAllowTextOverflow => {
self.state.line_width = 0;
continue;
}
Expand All @@ -1498,7 +1511,9 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}
}

if self.state.line_width > self.options().line_width.into() {
if self.state.line_width > self.options().line_width.into()
&& !args.measure_mode().allows_text_overflow()
{
return Fits::No;
}

Expand Down Expand Up @@ -1601,6 +1616,17 @@ enum MeasureMode {
/// The content only fits if none of the lines exceed the print width. Lines are terminated by either
/// a hard line break or a soft line break in [`PrintMode::Expanded`].
AllLines,

/// Measures all lines and allows lines to exceed the configured line width. Useful when it only matters
/// whether the content *before* and *after* fits.
AllLinesAllowTextOverflow,
}

impl MeasureMode {
/// Returns `true` if this mode allows text exceeding the configured line width.
const fn allows_text_overflow(self) -> bool {
matches!(self, MeasureMode::AllLinesAllowTextOverflow)
}
}

impl From<BestFittingMode> for MeasureMode {
Expand Down
1 change: 0 additions & 1 deletion crates/ruff_formatter/src/printer/printer_options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ impl SourceMapGeneration {
}
}

#[allow(dead_code)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum LineEnding {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,3 +394,15 @@
# c: and this comment
+ a
)

# Test for https://github.com/astral-sh/ruff/issues/7431
if True:
if True:
if True:
if True:
msg += " " + _(
"Since the role is not mentionable, it will be momentarily made mentionable "
"when announcing a streamalert. Please make sure I have the correct "
"permissions to manage this role, or else members of this role won't receive "
"a notification."
)
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,3 @@ def f():
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = (
True
)

# Regression test for https://github.com/astral-sh/ruff/issues/7462
if grid is not None:
rgrid = (rgrid.rio.reproject_match(grid, nodata=fillvalue) # rio.reproject nodata is use to initlialize the destination array
.where(~grid.isnull()))
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,19 @@ def test():
key9: value9,
}
), "Not what we expected and the message is too long to fit in one lineeeeeeeeeeeeeee"

# Test for https://github.com/astral-sh/ruff/issues/7246
assert items == [
"a very very very very very very very very very very very very very very very long string",
]

assert package.files == [
{
"file": "pytest-3.5.0-py2.py3-none-any.whl",
"hash": "sha256:6266f87ab64692112e5477eba395cfedda53b1933ccd29478e671e73b420c19c", # noqa: E501
},
{
"file": "pytest-3.5.0.tar.gz",
"hash": "sha256:fae491d1874f199537fd5872b5e1f0e74a009b979df9d53d1553fd03da1703e1", # noqa: E501
},
]
36 changes: 19 additions & 17 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,32 +155,34 @@ impl<'content, 'ast> FormatParenthesized<'content, 'ast> {

impl<'ast> Format<PyFormatContext<'ast>> for FormatParenthesized<'_, 'ast> {
fn fmt(&self, f: &mut Formatter<PyFormatContext<'ast>>) -> FormatResult<()> {
let inner = format_with(|f| {
let current_level = f.context().node_level();

let content = format_with(|f| {
group(&format_args![
token(self.left),
dangling_open_parenthesis_comments(self.comments),
soft_block_indent(&Arguments::from(&self.content)),
token(self.right)
soft_block_indent(&Arguments::from(&self.content))
])
.fmt(f)
});

let current_level = f.context().node_level();
let inner = format_with(|f| {
if let NodeLevel::Expression(Some(group_id)) = current_level {
// Use fits expanded if there's an enclosing group that adds the optional parentheses.
// This ensures that expanding this parenthesized expression does not expand the optional parentheses group.
write!(
f,
[fits_expanded(&content)
.with_condition(Some(Condition::if_group_fits_on_line(group_id)))]
)
} else {
// It's not necessary to wrap the content if it is not inside of an optional_parentheses group.
content.fmt(f)
}
});

let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f);

if let NodeLevel::Expression(Some(group_id)) = current_level {
// Use fits expanded if there's an enclosing group that adds the optional parentheses.
// This ensures that expanding this parenthesized expression does not expand the optional parentheses group.
write!(
f,
[fits_expanded(&inner)
.with_condition(Some(Condition::if_group_fits_on_line(group_id)))]
)
} else {
// It's not necessary to wrap the content if it is not inside of an optional_parentheses group.
write!(f, [inner])
}
write!(f, [token(self.left), inner, token(self.right)])
}
}

Expand Down

This file was deleted.

Loading

0 comments on commit 192463c

Please sign in to comment.