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

PEP 8: Add green and red sidbar for good and bad examples #3567

Merged
merged 3 commits into from
Dec 9, 2023

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Dec 5, 2023

As suggested in python/docs-community#22.

Like https://pep8.org, add green and red borders on the left side of "good" and "bad" code examples.

For example:

image image
image image

(This PR also removes redundant Version, Last-Modified and Content-Type headers, and Local Variables: footer.)

Accessibility check: colourblindness

We are not relying on colour to convey information; as before, the info about whether examples are good or bad is conveyed via the text.

We can later apply this to other good/bad code examples in other PEPs, such as the dangerous SQL code in PEP 675.

Question

I only added green/red for examples that are comparatively marked as correct or wrong (or similar). I left the other examples unmarked. But I think we could also mark those green, because they too are examples of good/allowed style. Thoughts?


📚 Documentation preview 📚: https://pep-previews--3567.org.readthedocs.build/pep-0008/

@hugovk hugovk added the infra Core infrastructure for building and rendering PEPs label Dec 5, 2023
@Rosuav
Copy link
Contributor

Rosuav commented Dec 5, 2023

Nice. I suspect this annotation can be put to use in quite a few places elsewhere too. Excellent enhancement IMO.

@hugovk
Copy link
Member Author

hugovk commented Dec 5, 2023

Question

I only added green/red for examples that are comparatively marked as correct or wrong (or similar). I left the other examples unmarked. But I think we could also mark those green, because they too are examples of good/allowed style. Thoughts?

Would look like this:

And give us extra green in places like:

@encukou
Copy link
Member

encukou commented Dec 6, 2023

Thank you! This looks useful for me as for a non-colourblind reader. I'm sure it could be made more accessible, perhaps in a dedicated PR.

I think we could also mark those green, because they too are examples of good/allowed style.

It does look better to me :)

IMO, in that case the outdated with example should be red (or left without colour): https://hugovk-peps.readthedocs.io/en/green-red2/pep-0008/#maximum-line-length
(perhaps with a comment that it's only for Python 3.10 and below)

Leave the multiple-with-statement-and-backslashes example uncoloured, it's for pre-3.10
@hugovk
Copy link
Member Author

hugovk commented Dec 8, 2023

Thanks, I've updated this PR to add those extra ones.

MO, in that case the outdated with example should be red (or left without colour): hugovk-peps.readthedocs.io/en/green-red2/pep-0008/#maximum-line-length
(perhaps with a comment that it's only for Python 3.10 and below)

Good point, I've left this one uncoloured:

https://pep-previews--3567.org.readthedocs.build/pep-0008/#maximum-line-length

@warsaw
Copy link
Member

warsaw commented Dec 8, 2023

Good point, I've left this one uncoloured

What about a yellow border for examples like this?

The diff is a little difficult to read, but I'll trust that there aren't any content changes. The red/green borders are very nice! Thanks for doing this.

@hugovk
Copy link
Member Author

hugovk commented Dec 8, 2023

Good point, I've left this one uncoloured

What about a yellow border for examples like this?

Something like this? https://pep-previews--3567.org.readthedocs.build/pep-0008/#maximum-line-length

image image

The diff is a little difficult to read, but I'll trust that there aren't any content changes. The red/green borders are very nice! Thanks for doing this.

Correct, no content changes.

Sphinx is stricter for .. code-block:: (needed for the class) than ::, so I had to adjust some indents.

@warsaw
Copy link
Member

warsaw commented Dec 8, 2023

Re: yellow - yes, exactly. It looks pretty good in both light and dark themes. Thanks!

@hugovk
Copy link
Member Author

hugovk commented Dec 9, 2023

Thanks for the reviews! Let's merge this, and we can then add to other PEPs where appropriate.

@hugovk hugovk merged commit d341072 into python:main Dec 9, 2023
23 checks passed
@hugovk hugovk deleted the green-red branch December 9, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Core infrastructure for building and rendering PEPs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants