Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@JustAnotherArchivist
Copy link
Contributor

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.

serhiy-storchaka reacted with thumbs up emoji
@bedevere-botbedevere-bot added awaiting review docsDocumentation in the Doc dir labelsJun 28, 2022
Copy link
Member

@serhiy-storchakaserhiy-storchaka left a 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.

@JustAnotherArchivist
Copy link
ContributorAuthor

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 whatthe 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.

@JustAnotherArchivistJustAnotherArchivistforce-pushed thedocs-contextlib-indentation-typo branch fromb983578 toe630b09CompareJune 28, 2022 18:22
@JustAnotherArchivistJustAnotherArchivist changed the titleFix indentation depth typo in contextlib.asynccontextmanager documentation exampleFix indentation in contextlib documentation to three spacesJun 28, 2022
@merwok
Copy link
Member

I don’t think the churn is worth it. This will not change the output, nor help developers.

rhettinger reacted with thumbs up emoji

@serhiy-storchaka
Copy link
Member

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.

@slateny
Copy link
Contributor

@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>>> from code examples as it doesn't help much with copying:

image

@merwok
Copy link
Member

Another possible change is to remove the>>> from code examples as it doesn't help much with copying:

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.

@slateny
Copy link
Contributor

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.

Not 100% on how it works, but in the case above it doesn't have this button in the top right:
image

I'm guessing that all of the code example would need the>>>/... for the button to be generated, otherwise it thinks it's a regular code block.

@slateny
Copy link
Contributor

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.

@slateny
Copy link
Contributor

See#98111

@slatenyslateny closed thisOct 9, 2022
@JustAnotherArchivist
Copy link
ContributorAuthor

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!

@slateny
Copy link
Contributor

No worries, thanks for the update 🙂

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka left review comments

@1st11st1Awaiting requested review from 1st1

Assignees

No one assigned

Labels

awaiting reviewdocsDocumentation in the Doc dirskip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@JustAnotherArchivist@merwok@serhiy-storchaka@slateny@bedevere-bot@AlexWaygood

[8]ページ先頭

©2009-2025 Movatter.jp