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

`should_use_best_fit with comments #7023

Closed
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
@@ -0,0 +1,14 @@
UNUSABLE_PASSWORD_SUFFIX_LENGTH = 40 # number of random chars to add after UNUSABLE_PASSWORD_PREFIX


UNUSABLE_PASSWORD_SUFFIX_LENGTH = (
40
) # number of random chars to add after UNUSABLE_PASSWORD_PREFIX



UNUSABLE_PASSWORD_SUFFIX_LENGTH = (
40 # number of random chars to add after UNUSABLE_PASSWORD_PREFIX
)


Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl FormatNodeRule<ExprConstant> for FormatExprConstant {
impl NeedsParentheses for ExprConstant {
fn needs_parentheses(
&self,
_parent: AnyNodeRef,
parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
if self.value.is_implicit_concatenated() {
Expand All @@ -86,7 +86,7 @@ impl NeedsParentheses for ExprConstant {
|| self.value.is_ellipsis()
{
OptionalParentheses::Never
} else if should_use_best_fit(self, context) {
} else if should_use_best_fit(self, parent, context) {
OptionalParentheses::BestFit
} else {
OptionalParentheses::Never
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ impl FormatNodeRule<ExprFString> for FormatExprFString {
impl NeedsParentheses for ExprFString {
fn needs_parentheses(
&self,
_parent: AnyNodeRef,
parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
if self.implicit_concatenated {
OptionalParentheses::Multiline
} else if memchr2(b'\n', b'\r', context.source()[self.range].as_bytes()).is_none()
&& should_use_best_fit(self, context)
&& should_use_best_fit(self, parent, context)
{
OptionalParentheses::BestFit
} else {
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_formatter/src/expression/expr_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ impl FormatNodeRule<ExprName> for FormatExprName {
impl NeedsParentheses for ExprName {
fn needs_parentheses(
&self,
_parent: AnyNodeRef,
parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
if should_use_best_fit(self, context) {
if should_use_best_fit(self, parent, context) {
OptionalParentheses::BestFit
} else {
OptionalParentheses::Never
Expand Down
12 changes: 10 additions & 2 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExpressionRef;
use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::Ranged;
use ruff_text_size::TextSize;

use crate::comments::{
dangling_comments, dangling_open_parenthesis_comments, trailing_comments, SourceComment,
Expand All @@ -29,11 +30,18 @@ pub(crate) enum OptionalParentheses {
Never,
}

pub(super) fn should_use_best_fit<T>(value: T, context: &PyFormatContext) -> bool
pub(super) fn should_use_best_fit<'a, T, U>(value: T, parent: U, context: &PyFormatContext) -> bool
where
T: Ranged,
U: Into<AnyNodeRef<'a>>,
{
let text_len = context.source()[value.range()].len();
let trailing_comments = context.comments().trailing(parent.into());
let eol_comment_length: TextSize = trailing_comments
.iter()
.filter(|c| c.line_position().is_end_of_line())
.map(|c| c.range().len())
.sum();

// Only use best fits if:
// * The text is longer than 5 characters:
Expand All @@ -44,7 +52,7 @@ where
// of 5 characters to avoid it exceeding the line width by 1 reduces the readability.
// * The text is know to never fit: The text can never fit even when parenthesizing if it is longer
// than the configured line width (minus indent).
text_len > 5
text_len + Into::<usize>::into(eol_comment_length) > 5
&& text_len
<= context.options().line_width().value() as usize
- context.options().indent_width() as usize
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/best_fit.py
---
## Input
```py
UNUSABLE_PASSWORD_SUFFIX_LENGTH = 40 # number of random chars to add after UNUSABLE_PASSWORD_PREFIX


UNUSABLE_PASSWORD_SUFFIX_LENGTH = (
40
) # number of random chars to add after UNUSABLE_PASSWORD_PREFIX


```

## Output
```py
UNUSABLE_PASSWORD_SUFFIX_LENGTH = (
40
) # number of random chars to add after UNUSABLE_PASSWORD_PREFIX


UNUSABLE_PASSWORD_SUFFIX_LENGTH = (
40
) # number of random chars to add after UNUSABLE_PASSWORD_PREFIX
```