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

ruff_python_formatter: add docstring-code-line-width internal setting #9055

Merged
merged 6 commits into from
Dec 11, 2023

Conversation

BurntSushi
Copy link
Member

@BurntSushi BurntSushi commented Dec 8, 2023

Summary

This does the light plumbing necessary to add a new internal option that permits setting the line width of code examples in docstrings. The plan is to add the corresponding user facing knob in #8854.

Note that this effectively removes the same-as-global configuration style discussed in this comment. It replaces it with the {integer} configuration style only.

There are a lot of commits here, but they are each tiny to make review easier because of the changes to snapshots.

Test Plan

I added a new docstring test configuration that sets docstring-code-line-width = 60 and examined the differences.

@BurntSushi BurntSushi added internal An internal refactor or improvement docstring Related to docstring linting or formatting formatter Related to the formatter labels Dec 8, 2023
This permits setting a line width for code examples in docstrings
that is distinct from the line width of the surrounding Python
code. Its default is the same as for Python: 88. But note that
changing the line width for the surrounding Python code does not
also change it for docstring code examples. They are configured
separately.

Note that I chose "width" for this internal option to make it
consistent with other internal options, even though it seems like
we would like to eventually use the word "length" to match public
facing documentation.
This adds the new docstring-code-line-width setting to the
snapshot output.
This applies the changes from the previous commit. We split it out into
a distinct commit to make review a little easier.
We add a new configuration with a docstring line width, but specifically
keep it at the default for now to make review changes based on line
width easier.

In the next commit, will add changes to the snapshot.

In the commit following that, we'll finally change the actual line width
setting and we'll be able to see precisely its impact.
This is the test output from the previous commit, where we set the
docstring code line width explicitly, but to the default.

The next commit will change the line width to a non-default setting.
This updates the line width setting and the test output. Since most of
our tests use very short lines, the chnages are small. They look like
what I'd expect.

Ref #8855
Copy link
Contributor

github-actions bot commented Dec 8, 2023

ruff-ecosystem results

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

/// docstring code examples. This only has an impact when `docstring_code`
/// is enabled.
#[cfg_attr(feature = "serde", serde(default = "default_line_width"))]
docstring_code_line_width: LineWidth,
Copy link
Member

Choose a reason for hiding this comment

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

I'd expect this to default to the formatter's line-length, but I think that happens at a level above, when we create the Settings.

Copy link
Member

@MichaReiser MichaReiser Dec 10, 2023

Choose a reason for hiding this comment

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

Good point. I don't know if serde supports defaults that depend on another field's value.

Copy link
Member Author

Choose a reason for hiding this comment

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

I specifically didn't attempt this, because this would imply a same-as-global setting which is distinct from a specific fixed integer. I think that if we do want that mode, then we ought to make it an explicit setting so that users can specifically opt into it in other contexts. So for example, instead of using LineWidth here, I'd do:

enum DocstringCodeLineWidth {
    SameAsGlobal,
    Fixed(LineWidth),
}

And then hopefully we'll add Dynamic (pending my experimentation today).

But we can also hash this out a bit more when I add the user-facing version of this.

@BurntSushi BurntSushi merged commit 07380e0 into main Dec 11, 2023
17 checks passed
@BurntSushi BurntSushi deleted the ag/fmt/line-length branch December 11, 2023 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docstring Related to docstring linting or formatting formatter Related to the formatter internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants