docstring code formatter: figure out how to handle line width #8855
Comments
BurntSushi
added
docstring
|
I would also very much prefer option 3 |
|
Do you know what rustfmt's experimental implementation of code-snipped formatting in the documentation does? Regarding optimising for the line width when presenting the example. I'm unfamiliar with Python documentation but assuming that markdown is supported, isn't it possible that examples could be nested inside lists or are part of other nested structures? In which case we couldn't determine the rendered width (although assuming that most code blocks aren't nested is probably true for the vast majority). |
|
I don't know that I have a strong opinion on what the "right behavior" is, but I would like to avoid adding an additional setting for this and instead make an opinionated decision between the two strategies -- which leads me to think we should just ship with what we have and react to user feedback if it comes in. |
|
There is definitely something to be said for the behavior you have implemented now. It will lead to the best experience for users browsing the docs when they have been built and published. However, from a developer standpoint, I will probably end up having to add I would like to at least have to option to enforce the resulting line length (in the file as opposed to the isolated example) - so that would be your option 3. This should probably be the default (or maybe an only option as per Charlie's comment)- it would also help users transition from other docstring formatters such as |
|
Another thing we need to be cognizant of is that rendered documentation almost always has a restricted horizontal viewport. For example, I prefer my line width to be 120 however I know that no documentation framework will do that. Here are some examples on my very large monitor:
After writing this out actually I changed my view a little bit and now I am quite confident that 3 should be the default behavior and 2 should be option so users can build for their particular theme. |
Good question. So rustfmt has two options here. In terms of its behavior, it seems like it operates the same as what we have today but also provides option (2). In particular, I went through the issues on the topic:
And the general sense that I got was that folks really wanted to be able to make code examples have a shorter line width than the surrounding code because the rendering usually uses less horizontal space overall. This drives folks to use a bit more wrapping than they otherwise would. It looks like @charliermarsh It looks to me like it would be somewhat difficult to do this without introducing another option, although I'm not opposed to shipping the status quo and seeing what kind of feedback we get. The status quo has no option, but that can lead to longer lines than one would like in the rendered docs and tripping linter warnings for long lines, forcing folks to either disable the lint or selectively ignore it (bummer). Doing (3) also lets us avoid adding an option, but it could result in much more tightly wrapped documentation examples than folks would like. |
|
One challenge I see with a new The downside I see with option 3 is that the width for examples will be different depending on the indent level at which they were defined. This may lead to inconsistency when viewing the documentation. However, it seems to be the only option that plays nicely with E501. |
|
If we're particularly concerned about conflicts with E501 or W505, should we consider adding a setting or exception for docstring code to the linter instead of complicating the formatter? |
|
I think in my ideal world, the code would by dynamically formatted to fit within my current line width depending on the indent of the block. While reading in an editor, the width would always be consistent. Later, documentation tooling would format the code blocks again at the desired width for display e.g. on an API reference website. |
I don't know of any docs framework that does this. As far as I know they leave it as-is and if the line is too long the browser will automatically add a horizontal scrollbar. |
|
It sounds like most folks want to make (3) our default behavior (so, use the same line width as the rest of the formatter, and take into account the docstring indentation), with a setting (2) to let you set a line width for docstring code that would ignore the indentation depth of the docstring itself. I have no objection to that, it seems reasonable to me. |
|
Although, is anyone going to use this setting described in (2)? It doesn't seem like anyone in this thread would actually use it. |
|
As I mentioned in my comment above, I would definitely use it because I prefer my line length to be 120 and my preferred documentation renders maximum ~80 characters and therefore I would set (2) to 80. edit: essentially I want to have readers never encounter a horizontal scrollbar in documentation |
We'll have to make one ;) although... this can be solved without a new tool if there's an option
You could use this option and reformat the project when generating the documentation without committing the change. It'd be nice to hear some more feedback requesting such a setting though. |
|
To be clear, you're saying that when we build docs the recommendation is to have a separate config file just for that option and format the code in the CI using that config file, and then run the documentation generator? |
|
Most of the time, I wouldn't mind that code examples in docstrings are formatted using the global line length, i.e. regardless of their base indentation, because docstrings are usually indented by 0, 1, or 2 levels, not more. 8 characters less is not a huge difference in size, at least when using a large line length such as 120. But if the line length is configured to 80, then it can become quite small yes, and pose readability and rendering issues. This would especially be problematic for (draft) PEP 727 docstrings (parameter docstrings using class Yo:
def yo(
param: Annotated[
str,
Doc(
"""
Here are code snippets.
- First:
```python
print("hello")
```
""",
),
],
):
...So in the end, I'd be more in favor of defaulting to a dynamic line length for code snippets in docstrings (configured line length + base indentation level), as well as allowing configuration of the line length for code snippets in docstrings, because as @ofek mentioned I think, while 120 is acceptable in the editor, SSGs will typically have a smaller rendering width like 80. Besides, configuring a smaller width for code examples in docstrings might help reducing false positive warnings on line length. For example:
|
Just to be totally clear, when you set (2) to 80, would you expect that to be 80 characters from the start of the line, or 80 characters from the start of the docstring (i.e., exclusive of the indentation)? |
|
80 characters from the start of the docstring |
Well ideally we'd make it easier than creating a whole separate config file to override a single option (as described in #8368) but yes. This is more of a solution for people who have a documentation line width that would cause conflict with their global line width e.g. they're the same number instead of 80 / 120. |
|
Bikeshed: what name should we use for the option to control line width in docstring code snippets? rustfmt uses Some ideas:
There is also the other question as to what the default value for this option should be. One thought is to make the default value |
|
My vote would be for My guess would be that |
Do you mean a string |
|
I was suggesting that we use the absence of a value to represent the default, and internally it would be modeled as the null value. But I guess that doesn't allow overrides for projects that use |
|
Yeah I was going to say that relying on "absence" to represent a value usually winds up breaking down for $reasons, with overrides and what not being one of them. You usually want some way to explicitly set a particular type of configuration, even if we do permit its absence to default to it. It also gives us wiggle room to add more values. I'm not opposed to a mixed type field, I just don't know enough about ruff's configuration yet to know whether that's both doable and desirable. I think the alternative is another config knob right? |
|
I also like |
I no longer see it as a priority to change from
The global line length setting defaults to 88 and not
I slightly prefer We decided to name the knob enabling docstring formatting
Implementing Considering that |
Yeah, I don't actually know precisely how to implement this. I probably naively assumed that I would "just" compute the indentation length of the minimum indent of the code block, subtract that from the configured global setting and use the result of that subtraction as the new global line width setting in the call to the formatter for the snippet. And of course, have some kind of reasonable fallback for cases where the new line width setting would be unreasonable small. I haven't actually tried that, so I don't know if it works. If it doesn't work, and we do indeed need to do something like what you said, then that could be an issue.
I think the issue with putting the onus on the documentation tooling is that it does let folks strike a practical middle ground when this feature ships. One of the key motivating factors for having
I am slightly confused by this phrasing. The status quo today is not To be clear, we could ship what we have today, as-is, with no new options. My instinct is that it will immediately lead to a bunch of complaints that we can predict. I don't know for sure though. Maybe it's fine. I think we need to at least provide a way to specify a fixed line width. It won't completely solve the user experience problem with violating line length limits, but it will probably let users heuristically avoid most issues. It also lets them set it to something small to motivate more wrapping in documentation without relying on their documentation tooling to do auto-reformatting itself. |
So to be extra clear here, one possibility is that we add a docstring code snippet line length option, set to a fixed default value (probably just |
I don't think we can rely on the source text for this, because the source text may not be correctly indented.
From what I understand is that On naming. ChatGPT is happy with
[tool.black]
line-length = 88
docstring-code-block-line-length = 80 |
Yeah exactly,
Right, I think I see now. I was only thinking about the contents of the docstring, but the indentation of the docstring itself might not be correct in the source. Is that what you mean? If so, yeah I'm not sure exactly how to implement I'll investigate what As for naming, I'm still note a huge fan of adding "block" in there. It feels a little redundant and more verbose? I'm not strongly opposed though. If everyone likes that naming, then I'm totally cool with it. |
|
OK, let's look at a concrete example to see how things behave. We'll start with this: def doctest_long_lines():
'''
Do cool stuff.
This won't get wrapped even though it exceeds our configured
line width because it doesn't exceed the line width within this
docstring. e.g, the `f` in `foo` is treated as the first column.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, bear)
But this one is long enough to get wrapped.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, monkey, spider, bear, leopard)
'''
# This demostrates a normal line that will get wrapped but won't
# get wrapped in the docstring above because of how the line-width
# setting gets reset at the first column in each code snippet.
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, monkey)The first doctest looks like this, and on its own does not exceed 88 columns (it's 86 columns): foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, bear)However, in the context of the source file, it looks like this, where it does exceed 88 columns (94 columns, including indentation): foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, bear)The second doctest exceeds 88 columns on its own (it's 111 columns): foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, monkey, spider, bear, leopard)Here's what def doctest_long_lines():
"""
Do cool stuff.
This won't get wrapped even though it exceeds our configured
line width because it doesn't exceed the line width within this
docstring. e.g, the `f` in `foo` is treated as the first column.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, bear)
But this one is long enough to get wrapped.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion, giraffe, hippo, zeba, lemur, penguin, monkey, spider, bear, leopard
)
"""
# This demostrates a normal line that will get wrapped but won't
# get wrapped in the docstring above because of how the line-width
# setting gets reset at the first column in each code snippet.
foo, bar, quux = this_is_a_long_line(
lion, giraffe, hippo, zeba, lemur, penguin, monkey
)The first doctest, despite it exceeding the line length of 88, does not get wrapped. This is because the actual code itself does not exceed the line length. The second doctest does however, since it is long enough on its own to exceed the global configured line width. So, how do For def doctest_long_lines():
'''
Do cool stuff.
This won't get wrapped even though it exceeds our configured
line width because it doesn't exceed the line width within this
docstring. e.g, the `f` in `foo` is treated as the first column.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion, giraffe, hippo, zeba, lemur, penguin, bear
)
But this one is long enough to get wrapped.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion, giraffe, hippo, zeba, lemur, penguin, monkey, spider, bear, leopard
)
'''
# This demostrates a normal line that will get wrapped but won't
# get wrapped in the docstring above because of how the line-width
# setting gets reset at the first column in each code snippet.
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, monkey)Notice that the first doctest is wrapped, so it seems like And for def doctest_long_lines():
'''
Do cool stuff.
This won't get wrapped even though it exceeds our configured
line width because it doesn't exceed the line width within this
docstring. e.g, the `f` in `foo` is treated as the first column.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, bear)
But this one is long enough to get wrapped.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion, giraffe, hippo, zeba, lemur, penguin, monkey, spider, bear, leopard
)
'''
# This demostrates a normal line that will get wrapped but won't
# get wrapped in the docstring above because of how the line-width
# setting gets reset at the first column in each code snippet.
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, monkey)So it looks like Where we now get: def doctest_long_lines():
'''
Do cool stuff.
This won't get wrapped even though it exceeds our configured
line width because it doesn't exceed the line width within this
docstring. e.g, the `f` in `foo` is treated as the first column.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion, giraffe, hippo, zeba, lemur, penguin, bear
)
But this one is long enough to get wrapped.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion,
giraffe,
hippo,
zeba,
lemur,
penguin,
monkey,
spider,
bear,
leopard,
)
'''
# This demostrates a normal line that will get wrapped but won't
# get wrapped in the docstring above because of how the line-width
# setting gets reset at the first column in each code snippet.
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, monkey)And, notably, Where we now get the same as def doctest_long_lines():
'''
Do cool stuff.
This won't get wrapped even though it exceeds our configured
line width because it doesn't exceed the line width within this
docstring. e.g, the `f` in `foo` is treated as the first column.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion, giraffe, hippo, zeba, lemur, penguin, bear
)
But this one is long enough to get wrapped.
.. code-block:: python
foo, bar, quux = this_is_a_long_line(
lion,
giraffe,
hippo,
zeba,
lemur,
penguin,
monkey,
spider,
bear,
leopard,
)
'''
# This demostrates a normal line that will get wrapped but won't
# get wrapped in the docstring above because of how the line-width
# setting gets reset at the first column in each code snippet.
foo, bar, quux = this_is_a_long_line(lion, giraffe, hippo, zeba, lemur, penguin, monkey)So, let's categorize things. I think there are three known different configuration setups here:
So that means:
|
|
I am not fully comfortable shipping with Note that it may be the case that we just never need to provide a |
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
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
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
…#9055) ## 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](#8855 (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.
This computes the line width to use on the nested call to the formatter based on the current indent level, indent width and configured global setting. Ref #8855
This computes the line width to use on the nested call to the formatter based on the current indent level, indent width and configured global setting. Ref #8855
This computes the line width to use on the nested call to the formatter based on the current indent level, indent width and configured global setting. Ref #8855
This computes the line width to use on the nested call to the formatter based on the current indent level, indent width and configured global setting. Ref #8855
|
With #9098 merged, I think we've settled on our initial strategy here. We'll provide two ways to configure the line width for docstring code snippets:
|
In #8811, the formatter grew the ability to format doctest code snippets in docstrings. In this initial implementation, it handles line width limits by resetting the column of the code snippet to the first column. It is unclear whether this is the right behavior. This means that individual lines inside the docstring can exceed line length limits imposed by the linter, and it also just generally means that it is difficult to maintain a line length limit in the source code.
The benefit of the current behavior is that it prioritizes the presentation of the code snippet itself. So that when a documentation generator renders a code snippet, its presented line length will be correct with respect to the project's configured line limits.
The downside of the current behavior is that the source code itself can have long lines. (This would be quite annoying to me personally, perhaps enough to eclipse the benefit of the presentation having a correct line length.)
Here are a few options we can pursue:
docstring-code-line-width, that lets one configure the line width of code snippets independent of the line width of the surrounding code.docstring-code-preserve-line-widththat dynamically chooses the line width of a reformatted code snippet based on the column at which in starts within the source code and the globally configured line width.Maybe there are other choices. Speaking for myself personally, if I were to use code snippet formatting, I would want option (3) because it preserves the line length limit correctly regardless of the indentation of the docstring. I'm not sure I would add option (2) at all.
The text was updated successfully, but these errors were encountered: