Skip to content

gh-99087: Add missing newline for prompts in docs#98993

Merged
ethanfurman merged 2 commits into
python:mainfrom
slateny:s/newline-code
Dec 9, 2022
Merged

gh-99087: Add missing newline for prompts in docs#98993
ethanfurman merged 2 commits into
python:mainfrom
slateny:s/newline-code

Conversation

@slateny
Copy link
Copy Markdown
Contributor

@slateny slateny commented Nov 2, 2022

@erlend-aasland
Copy link
Copy Markdown
Contributor

This is against the recommendations by the dev guide: https://devguide.python.org/documentation/style-guide/#code-examples

Suggesting to close this PR.

@erlend-aasland erlend-aasland added the pending The issue will be closed if no feedback is provided label Nov 2, 2022
@slateny
Copy link
Copy Markdown
Contributor Author

slateny commented Nov 2, 2022

I would argue that it does aid copy-pasting and is necessary despite impeding some readability: for example, in the enum howto, we have this:
image

which (when the >>> is clicked on the top-right of the code box) corresponds to the prompt

class Perm(IntFlag):
    R = 4
    W = 2
    X = 1
    RWX = 7
Perm.RWX

~Perm.RWX

Perm(7)

but this causes a error when pasted:

>>> class Perm(IntFlag):
...     R = 4
...     W = 2
...     X = 1
...     RWX = 7
... Perm.RWX
  File "<stdin>", line 6
    Perm.RWX
    ^^^^
SyntaxError: invalid syntax

This happens if the previous line is a part of something function, class, etc. when there's indentation, so the new line's needed to clear it out. I should have also put my justification in the description instead of leaving it empty - my bad on that.

@JelleZijlstra
Copy link
Copy Markdown
Member

I think the devguide paragraph @erlend-aasland cites isn't exactly on point here. It's about not introducing the secondary prompt to examples that currently don't have it, but this PR adds an extra line of secondary prompt to examples that already have it.

@erlend-aasland
Copy link
Copy Markdown
Contributor

I should have also put my justification in the description instead of leaving it empty - my bad on that.

Yeah, either that, or creating an issue describing the problem. Perhaps an issue would have been appropriate here, since it is clearly not a trivial change.

@rhettinger
Copy link
Copy Markdown
Contributor

rhettinger commented Nov 2, 2022

The problem here is that the edit looks gross and makes the examples slightly longer, noisier, and less readable without adding anything substantive.

@slateny
Copy link
Copy Markdown
Contributor Author

slateny commented Nov 3, 2022

The problem here is that the edit looks gross and makes the examples slightly longer, noisier, and less readable without adding anything substantive.

Preventing errors from copy-pasting prompts, especially from howto pages, would quality as substantive to me, and since the edit is just extending the existing ... it doesn't appear that much visually intrusive. However, if the consensus is that prompts are to be left as-is intentionally, then I have no problem with closing this PR and pointing future PRs with similar changes to this one for justification.

@slateny slateny changed the title Add missing newline for prompts in docs gh-99087: Add missing newline for prompts in docs Nov 4, 2022
Copy link
Copy Markdown
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

I think this change is helpful, though I must admit I'd never even noticed the button to remove the prompts from code samples. I won't merge it though until there's consensus on the issue.

@erlend-aasland
Copy link
Copy Markdown
Contributor

I think this change is helpful [...]

I agree, it makes sense to me. I also think we should amend the dev guide.

@erlend-aasland erlend-aasland removed the pending The issue will be closed if no feedback is provided label Nov 27, 2022
@erlend-aasland
Copy link
Copy Markdown
Contributor

FTR; the sqlite3 change is no longer relevant.

@netlify
Copy link
Copy Markdown

netlify Bot commented Dec 8, 2022

Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit db5bbe3
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/63926a89e5beed0009ff7d1c
😎 Deploy Preview https://deploy-preview-98993--python-cpython-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@ethanfurman ethanfurman merged commit 286e3c7 into python:main Dec 9, 2022
@slateny slateny deleted the s/newline-code branch December 9, 2022 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants