Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix codeblock dynamic line length calculation for indented examples #13523

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,72 @@ def doctest_extra_indent3():
... df1, df2, df3, on="dt"
... ) # doctest: +IGNORE_RESULT
"""

# See https://github.com/astral-sh/ruff/issues/13358
def length_doctest():
"""Get the length of the given list of numbers.

Args:
numbers: List of numbers.

Returns:
Integer length of the list of numbers.

Example:
>>> length([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20])
20
"""


def length_doctest_underindent():
"""Get the length of the given list of numbers.

Args:
numbers: List of numbers.

Returns:
Integer length of the list of numbers.

Example:
>>> length([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20])
20
"""


# See https://github.com/astral-sh/ruff/issues/13358
def length_markdown():
"""Get the length of the given list of numbers.

Args:
numbers: List of numbers.

Returns:
Integer length of the list of numbers.

Example:

```
length([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21])
```
"""


# See https://github.com/astral-sh/ruff/issues/13358
def length_rst():
"""
Do cool stuff::

length([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21])
"""
pass


# See https://github.com/astral-sh/ruff/issues/13358
def length_rst_in_section():
"""
Examples:
Do cool stuff::

length([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20])
"""
pass
9 changes: 9 additions & 0 deletions crates/ruff_python_formatter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,12 @@ pub(crate) fn is_empty_parameters_no_unnecessary_parentheses_around_return_value
pub(crate) fn is_match_case_parentheses_enabled(context: &PyFormatContext) -> bool {
context.is_preview()
}

/// This preview style fixes a bug with the docstring's `line-length` calculation when using the `dynamic` mode.
/// The new style now respects the indent **inside** the docstring and reduces the `line-length` accordingly
/// so that the docstring's code block fits into the global line-length setting.
pub(crate) fn is_docstring_code_block_in_docstring_indent_enabled(
context: &PyFormatContext,
) -> bool {
context.is_preview()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this is treated as a new style instead of a bug fix? (Sorry, I'm probably out of step with what our policy is here.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatter versioning policy is rather strict:

The stable style changed to prevent invalid syntax, changes to the program's semantics, or removal of comments

It only allows hard correctness errors. The only other permitted changes are bug fixes that don't change any already formatted code. For example, we can fix a bug where the formatter incorrectly collapses two blank lines to one instead of preserving one or two.

The motivation for this strict rule is that a ruff format --check command shouldn't start failing between patch versions for already formatted code.

67 changes: 50 additions & 17 deletions crates/ruff_python_formatter/src/string/docstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ use {
ruff_text_size::{Ranged, TextLen, TextRange, TextSize},
};

use super::NormalizedString;
use crate::preview::is_docstring_code_block_in_docstring_indent_enabled;
use crate::string::StringQuotes;
use crate::{prelude::*, DocstringCodeLineWidth, FormatModuleError};

use super::NormalizedString;

/// Format a docstring by trimming whitespace and adjusting the indentation.
///
/// Summary of changes we make:
Expand Down Expand Up @@ -189,7 +189,7 @@ pub(crate) fn format(normalized: &NormalizedString, f: &mut PyFormatter) -> Form
// We don't want to count whitespace-only lines as miss-indented
.filter(|line| !line.trim().is_empty())
.map(Indentation::from_str)
.min_by_key(|indentation| indentation.width())
.min_by_key(|indentation| indentation.columns())
.unwrap_or_default();

DocstringLinePrinter {
Expand Down Expand Up @@ -353,7 +353,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
};
// This looks suspicious, but it's consistent with the whitespace
// normalization that will occur anyway.
let indent = " ".repeat(min_indent.width());
let indent = " ".repeat(min_indent.columns());
for docline in formatted_lines {
self.print_one(
&docline.map(|line| std::format!("{indent}{line}")),
Expand All @@ -363,7 +363,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
CodeExampleKind::Markdown(fenced) => {
// This looks suspicious, but it's consistent with the whitespace
// normalization that will occur anyway.
let indent = " ".repeat(fenced.opening_fence_indent.width());
let indent = " ".repeat(fenced.opening_fence_indent.columns());
for docline in formatted_lines {
self.print_one(
&docline.map(|line| std::format!("{indent}{line}")),
Expand Down Expand Up @@ -455,7 +455,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
// (see example in [`format_docstring`] doc comment). We then
// prepend the in-docstring indentation to the string.
let indent_len =
Indentation::from_str(trim_end).width() - self.stripped_indentation.width();
Indentation::from_str(trim_end).columns() - self.stripped_indentation.columns();
let in_docstring_indent = " ".repeat(indent_len) + trim_end.trim_start();
text(&in_docstring_indent).fmt(self.f)?;
};
Expand Down Expand Up @@ -500,11 +500,24 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
let global_line_width = self.f.options().line_width().value();
let indent_width = self.f.options().indent_width();
let indent_level = self.f.context().indent_level();
let current_indent = indent_level
let mut current_indent = indent_level
.to_ascii_spaces(indent_width)
.saturating_add(kind.extra_indent_ascii_spaces());

if is_docstring_code_block_in_docstring_indent_enabled(self.f.context()) {
// Add the in-docstring indentation
current_indent = current_indent.saturating_add(
u16::try_from(
kind.indent()
.columns()
.saturating_sub(self.stripped_indentation.columns()),
)
.unwrap_or(u16::MAX),
);
}

let width = std::cmp::max(1, global_line_width.saturating_sub(current_indent));
LineWidth::try_from(width).expect("width is capped at a minimum of 1")
LineWidth::try_from(width).expect("width should be capped at a minimum of 1")
}
};

Expand Down Expand Up @@ -828,6 +841,26 @@ impl<'src> CodeExampleKind<'src> {
_ => 0,
}
}

/// The indent of the entire code block relative to the start of the line.
///
/// For example:
/// ```python
/// def test():
/// """Docstring
/// Example:
/// >>> 1 + 1
/// ```
///
/// The `>>> ` block has an indent of 8 columns: The shared indent with the docstring and the 4 spaces
/// inside the docstring.
fn indent(&self) -> Indentation {
match self {
CodeExampleKind::Doctest(doctest) => Indentation::from_str(doctest.ps1_indent),
CodeExampleKind::Rst(rst) => rst.min_indent.unwrap_or(rst.opening_indent),
CodeExampleKind::Markdown(markdown) => markdown.opening_fence_indent,
}
}
}

/// State corresponding to a single doctest code example found in a docstring.
Expand Down Expand Up @@ -1663,7 +1696,7 @@ impl Indentation {
/// to the next multiple of 8. This is effectively a port of
/// [`str.expandtabs`](https://docs.python.org/3/library/stdtypes.html#str.expandtabs),
/// which black [calls with the default tab width of 8](https://github.com/psf/black/blob/c36e468794f9256d5e922c399240d49782ba04f1/src/black/strings.py#L61).
const fn width(self) -> usize {
const fn columns(self) -> usize {
match self {
Self::Spaces(count) => count,
Self::Tabs(count) => count * Self::TAB_INDENT_WIDTH,
Expand Down Expand Up @@ -1769,7 +1802,7 @@ impl Indentation {
fn trim_start_str(self, line: &str) -> &str {
let mut seen_indent_len = 0;
let mut trimmed = line;
let indent_len = self.width();
let indent_len = self.columns();

for char in line.chars() {
if seen_indent_len >= indent_len {
Expand Down Expand Up @@ -1797,13 +1830,13 @@ impl Indentation {

impl PartialOrd for Indentation {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.width().cmp(&other.width()))
Some(self.columns().cmp(&other.columns()))
}
}

impl PartialEq for Indentation {
fn eq(&self, other: &Self) -> bool {
self.width() == other.width()
self.columns() == other.columns()
}
}

Expand Down Expand Up @@ -1843,10 +1876,10 @@ mod tests {
use crate::string::docstring::Indentation;

#[test]
fn test_indentation_like_black() {
assert_eq!(Indentation::from_str("\t \t \t").width(), 24);
assert_eq!(Indentation::from_str("\t \t").width(), 24);
assert_eq!(Indentation::from_str("\t\t\t").width(), 24);
assert_eq!(Indentation::from_str(" ").width(), 4);
fn indentation_like_black() {
assert_eq!(Indentation::from_str("\t \t \t").columns(), 24);
assert_eq!(Indentation::from_str("\t \t").columns(), 24);
assert_eq!(Indentation::from_str("\t\t\t").columns(), 24);
assert_eq!(Indentation::from_str(" ").columns(), 4);
}
}
Loading
Loading