-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
Fix indentation in contextlib documentation to three spaces #94361
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 indentation in contextlib documentation to three spaces #94361
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be more consistent to increase the indentation in other part of the example.
Oh, good point, I didn't compare it to the other code blocks in that file. Yeah, I suppose it would be slightly more consistent, although I see two, three, four, and five spaces of indentation relative to the preceding paragraphs... It seems like three is the most common though, and that's what the dev guide says as well (three spaces in reST, 4 spaces inside the Python blocks, but the indentation of the code blocks would be part of reST syntax). I'll normalise the entire document to use three for consistency. |
b983578
to
e630b09
Compare
I don’t think the churn is worth it. This will not change the output, nor help developers. |
I mostly agree with @merwok -- the churn is too large, and in most cases it does not affect the output. But in two cases, in the original code example and in the versionchanged directive for asynccontextmanager, incorrect indentation affects the output. It is a bug which should be fixed. I may miss other such cases, please re-check me. |
@JustAnotherArchivist Are you still interested in combing through the docs for whitespace / visible formatting changes? I think the important ones listed here are the original stray space and also some three-space indentation in code examples. Another possible change is to remove the |
I think that’s been addressed quite some time ago: there is now a formatting that adds buttons to toggle prompts on/off, and easily copy the code block. |
I'll start on a new PR here then and close this when that one's open - let me know if you'd still like to continue this one. |
See #98111 |
Apologies, I hadn't seen the notification emails for this for some reason. I disagree with the 'churn' argument, but that's a bit late now. Thanks for the fix, @slateny! |
No worries, thanks for the update 🙂 |
There was a stray space at the beginning of some lines in the async
@timeit
example.As this is a trivial typo fix, no issue exists, and a news entry shouldn't be necessary.