Skip to content

Roles/styles for good/bad code blocks in PEPs #22

Closed
python/peps
#3574

Description

@encukou

Do we want to add styling for good/bad code blocks to PEPs?

For an epxample, look at pep8.org, a third-party (and outdated) rendering of PEP 8
The main difference between it and the new style proposed for PEPs is the red & green indicators on what to do vs. what not to do.

I've just read the dangerous "naive" SQL code in PEP-0675 and thought it could use a similar "don't copy this" indicator. There surely are many other examples.
Of course everything should be covered in prose for screen readers and color-blind people, but for many people colors could aid understanding.

Would it be useful in docs as well?

Activity

  1. AA-Turner commented on Feb 15, 2022

    @AA-Turner
    Member

    This would be reasonably easy with

    .. code:: python
       :class: bad
    

    & similar for good -- it is then just a stylesheet change.

    We could add .. code-good:: and .. code-bad:: directives, but I think the first example is better as it requires no change to the parsing, so will be picked up by e.g. GitHub's RST renderer.

    A

  2. encukou commented on Feb 16, 2022

    @encukou
    MemberAuthor

    I don't know much about where it should be discussed -- perhaps Discourse would be better.
    But IMO it would be good to bring it up after/if PEP 676 (PEP Infrastructure Process) lands, so the change is actually visible, and docs about it can be added to PEP 12 (Sample reStructuredText PEP Template).

  3. CAM-Gerlach commented on Feb 16, 2022

    @CAM-Gerlach
    Member

    Sounds like a great change to me, and @AA-Turner 's approach makes sense—so long as there is a way to clearly communicate to doc writers how and when they should use it (or is this just for PEPs?)

    Also, @AA-Turner , any reason to prefer .. code:: vs. .. code-block::, especially if this is deferred until after PEP 676?

  4. pradyunsg commented on Feb 16, 2022

    @pradyunsg
    Member

    They're aliases, so there isn't any functional difference between the two.

  5. CAM-Gerlach commented on Feb 16, 2022

    @CAM-Gerlach
    Member

    They're aliases, so there isn't any functional difference between the two.

    Hmm, well then if that's the case, the Sphinx and the Docutils documentation should be updated, then—Docutils only lists .. code (though .. code-block, at least without options, appears to work just fine in practice on the legacy PEP builder, just without syntax highlighting—perhaps Docutils has added support or @AA-Turner added it as a custom directive), which only has three allowed options (number-lines, plus name and class), while Sphinx's docs only list .. code-block with only sourcecode as an alias, which has a plethora of options but does not include number-lines, which is instead named linenos; nor class, which is instead recommended to be added with Sphinx's .. rst-class:: directive instead—and thus, if the docs are correct, is the answer to my question as to why @AA-Turner used .. code:: instead, for compatibility with the legacy docutils builder.

    If we instead defer this to after if/when PEP 676 is adopted, as @encukou suggests and we seem to agree on, we can use the modern Sphinx semantics with .. code-block:: and .. rst-class instead of the legacy .. code:: and :class:.

  6. pradyunsg commented on Feb 16, 2022

    @pradyunsg
    Member
  7. pradyunsg commented on Feb 16, 2022

    @pradyunsg
    Member

    I personally think using :class: on .. code-block is a good idea, but let's cross that bridge when we get to it. Let's wait on PEP 676 to get decided on. :)

  8. AA-Turner commented on Feb 16, 2022

    @AA-Turner
    Member

    code-block works with pure Docutils, check the language aliases.

    Will update when on PC.

    A

  9. pradyunsg commented on Feb 16, 2022

    @pradyunsg
    Member

    Indeed! There's an en.py that has:

          'code': 'code',
          'code-block': 'code',
          'sourcecode': 'code',
    

    I should've looked closer. I guess my 20-ish year old brain's memory isn't completely trash yet. :)

    Sphinx also has a code directive -- https://github.com/sphinx-doc/sphinx/blob/8b23f6db12d9fcdb6e1ca2015ea32e3e2e1fbdb6/sphinx/directives/patches.py#L147

  10. pradyunsg commented on Feb 16, 2022

    @pradyunsg
    Member

    So... final update:

    • code, code-block and sourcecode work with both Sphinx and docutils.
    • They're all aliases, in docutils, for English content.
    • code is different-ish from code-block/sourcecode in Sphinx -- you can emphasis line numbers and do a couple more things with code-block but not code.
  11. CAM-Gerlach commented on Feb 16, 2022

    @CAM-Gerlach
    Member

    I personally think using :class: on .. code-block is a good idea, but let's cross that bridge when we get to it. Let's wait on PEP 676 to get decided on. :)

    I'm assuming you mean .. rst-class with .. code-block? The .. code-block:: directive doesn't have the legacy :class: option like .. code::, as that is now handled with the modern .. rst-class directive with Sphinx (confirmed via testing).

    But otherwise, 👍 to the above.

  12. pradyunsg commented on Feb 17, 2022

    @pradyunsg
    Member

    The .. code-block:: directive doesn't have the legacy :class: option like .. code::,

    It does have that tho. https://github.com/sphinx-doc/sphinx/blob/a55a765e797197cfdf8a3e5033efe8ed72191a62/sphinx/directives/code.py#L114

    Also, I'm not really sure why you consider passing an option to a directive to be "legacy" -- could you elaborate on why you think that's "legacy"?

  13. CAM-Gerlach commented on Feb 17, 2022

    @CAM-Gerlach
    Member

    It does have that tho. https://github.com/sphinx-doc/sphinx/blob/a55a765e797197cfdf8a3e5033efe8ed72191a62/sphinx/directives/code.py#L114

    Hmm, well upon further testing it does appear to work (I somehow missed seeing the class on the outer div), but it appears to be undocumented (despite :name: being fully documented), likely for backward-compat with docutils, nor is it documented on any other Sphinx directives or elsewhere. Also, on both docutils and Sphinx, aligning the :class: option with the beginning of the directive but then using standard four-space indents for the block content injects an extra pre-pended space into the rendered output.

    Also, I'm not really sure why you consider passing an option to a directive to be "legacy" -- could you elaborate on why you think that's "legacy"?

    Its not that passing an option to a directive in the general case is "legacy", but rather this specific case of a bespoke :class: option to this specific directive seems to be, given it is undocumented while elsewhere the documented, standard method of applying a class to any reST element is the rst-class directive, which also avoids the aforementioned rendering issue when using standard 4-space code indents but 3-space directive option spacing.

  14. AA-Turner commented on Feb 17, 2022

    @AA-Turner
    Member

    bespoke :class: option

    generally with reST you can pass :class: to any directive (entirely from memory, untested, and still on mobile so won't search for documentation). Note also the evolution of code and code-block is odd, with Sphinx implementing code-block first and Docutils later adding code.

    A

  15. changed the title [-]Roles/styles for good/bad code blocks[/-] [+]Roles/styles for good/bad code blocks in PEPs[/+] on May 3, 2022
  16. encukou commented on Dec 4, 2023

    @encukou
    MemberAuthor
  17. hugovk commented on Dec 5, 2023

    @hugovk
    Member

    PEP 676 has landed, please see PR python/peps#3567 to add green/red borders to the left of good/bad code examples of PEP 8.

    We can add them to other PEPs afterwards.

  18. hugovk commented on Dec 5, 2023

    @hugovk
    Member

    For the examples of good/bad commits in the devguide: python/devguide#1237

  19. hugovk commented on Dec 9, 2023

    @hugovk
    Member

    python/devguide#1237 has been merged to add the CSS classes and apply them to PEP 8.

    I've just read the dangerous "naive" SQL code in PEP-0675 and thought it could use a similar "don't copy this" indicator. There surely are many other examples.

    Please see python/peps#3574 to add a red sidebar here.

    We can close this issue now the CSS is available, we can add to other PEPs as and when. 🚀

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions