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.

Sign up for GitHub

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

Jump to bottom

Syntax highlighting for ASDL #84697

Closed
isidentical opened this issue May 5, 2020 · 22 comments
Closed

Syntax highlighting for ASDL #84697

isidentical opened this issue May 5, 2020 · 22 comments
Labels
3.9 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@isidentical
Copy link
Sponsor Member

BPO 40517
Nosy @rhettinger, @terryjreedy, @ezio-melotti, @merwok, @zware, @willingc, @JulienPalard, @isidentical
PRs
  • bpo-40517: Implement syntax highlighting support for ASDL #19928
  • Revert "bpo-40517: Implement syntax highlighting support for ASDL" #19950
  • bpo-40517: Implement syntax highlighting support for ASDL #19967
    		•
    Files
  • Screen Shot 2020-05-05 at 10.17.15 PM.png: Screen shot of the AST docs
  • asdl.png
  • asdl2.png
  • python_bold.png
  • asdl_no_style.png
  • bold_dark_blue.png
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2020-05-07.20:58:17.968>
created_at = <Date 2020-05-05.13:51:02.634>
labels = ['type-feature', '3.9', 'docs']
title = 'Syntax highlighting for ASDL'
updated_at = <Date 2020-05-07.21:01:28.137>
user = 'https://github.com/isidentical'

    bugs.python.org fields:

    activity = <Date 2020-05-07.21:01:28.137>
actor = 'BTaskaya'
assignee = 'docs@python'
closed = True
closed_date = <Date 2020-05-07.20:58:17.968>
closer = 'rhettinger'
components = ['Documentation']
creation = <Date 2020-05-05.13:51:02.634>
creator = 'BTaskaya'
dependencies = []
files = ['49129', '49133', '49134', '49135', '49136', '49138']
hgrepos = []
issue_num = 40517
keywords = ['patch']
message_count = 22.0
messages = ['368148', '368209', '368225', '368226', '368228', '368230', '368266', '368291', '368292', '368294', '368297', '368300', '368303', '368311', '368313', '368324', '368331', '368344', '368352', '368376', '368377', '368378']
nosy_count = 9.0
nosy_names = ['rhettinger', 'terry.reedy', 'ezio.melotti', 'eric.araujo', 'docs@python', 'zach.ware', 'willingc', 'mdk', 'BTaskaya']
pr_nums = ['19928', '19950', '19967']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40517'
versions = ['Python 3.9']
    @isidentical
    Copy link
    Sponsor Member Author

    ASDL is around here for a long time, and it was showed as raw text on documentation (under library/ast), IMHO it would be great to highlight it.

    @isidentical isidentical added docs Documentation in the Doc dir type-feature A feature request or enhancement labels May 5, 2020
    @isidentical isidentical added docs Documentation in the Doc dir type-feature A feature request or enhancement labels May 5, 2020
    @rhettinger
    Copy link
    Contributor

    +1

    @rhettinger
    Copy link
    Contributor

    New changeset d60040b by Batuhan Taskaya in branch 'master':
    bpo-40517: Implement syntax highlighting support for ASDL (bpo-19928)
    d60040b

    @rhettinger
    Copy link
    Contributor

    New changeset eff870b by Raymond Hettinger in branch 'master':
    Revert "bpo-40517: Implement syntax highlighting support for ASDL (bpo-19928)" (bpo-19950)
    eff870b

    @rhettinger
    Copy link
    Contributor

    I really like this idea but it needs different styling. Attaching a screen shot will significant readability and beauty issues.

    Suggest:

    • Boldfacethe class names (Module, Interactive, etc)
    • Unboldface the fields names (body, types_ignores, argtypes, etc)
    • The tinted grape color does not work well on the green background.
    • Perhaps have a graphic designer take a look.

    @isidentical
    Copy link
    Sponsor Member Author

    Oh, sorry for that bad look :/ I just want it to look consistent, let me see what I can do further (I'll probably consult a friend of mine who understand this things). By the way, I could've just adjust the values over the existing code if you didn't revert, the core logic would be same we just need to change token types.

    @rhettinger
    Copy link
    Contributor

    Sorry about the commit/revert. I fat fingered a comment. Please do resubmit the PR. In general, this is a nice idea. The look just needs to be tweaked a bit.

    @isidentical
    Copy link
    Sponsor Member Author

    Hey Raymond, can you give me your feedback on asdl.png (the screenshot of new theme)?

    @rhettinger
    Copy link
    Contributor

    The new screenshot looks nice. The colors are much better. Can you post another run with the class names in bold.

    @isidentical
    Copy link
    Sponsor Member Author

    Unfortunately there aren't many bold type tokens I can use, so I had to change color of module . If you wish I can make both class names and Python bold, or keep it in this way.

    @isidentical
    Copy link
    Sponsor Member Author

    (by the way, I did not push this change. I'll push it when you think it is ready)

    @terryjreedy
    Copy link
    Member

    I don't like the red Python in asdl2. Just black, perhaps bold, would be better. Also I like the darker blue in asdl.py, bold or not. Better contrast to me from the greens. But I agree that grape is too clashy.

    @isidentical
    Copy link
    Sponsor Member Author

    Attaching 2 different styles python_bold.png (module <Python> is bold) and asdl_no_style.png (module <Python> has no style)

    @terryjreedy
    Copy link
    Member

    I tried both a laptop and desktop and slightly prefer unbolded. How about a darker blue?

    @isidentical
    Copy link
    Sponsor Member Author

    I tried both a laptop and desktop and slightly prefer unbolded. How about a darker blue?

    Sorry but I have no control over styles. They are pre-defined, I only change the token type and pygments handles the rest of it. I dont know if such a color exists.

    @rhettinger
    Copy link
    Contributor

    Thanks for producing the comparison panel. In side-by-side views, python_bold.png looks best to me with asdl2.png as a close second.

    @isidentical
    Copy link
    Sponsor Member Author

    I've found a bold dark blue, which I guess suits both your and @terry.reedy recommendations. How does bold_dark_blue.png looks?

    @zware
    Copy link
    Member

    zware commented May 7, 2020

    I say merge it :)

    @isidentical
    Copy link
    Sponsor Member Author

    I've updated the PR with bold_dark_blue.png changes.

    @rhettinger
    Copy link
    Contributor

    New changeset b7a78ca by Batuhan Taskaya in branch 'master':
    bpo-40517: Implement syntax highlighting support for ASDL (GH-19967)
    b7a78ca

    @rhettinger
    Copy link
    Contributor

    Thanks for the contribution. It looks much nicer than what we had before.

    @rhettinger rhettinger added the 3.9 only security fixes label May 7, 2020
    @rhettinger rhettinger added the 3.9 only security fixes label May 7, 2020
    @isidentical
    Copy link
    Sponsor Member Author

    Thank you all for your reviews for styling, also I have to thank https://github.com/CyberSaxosTiGER for his external reviews on the color scheme.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @rhettinger @zware @terryjreedy @isidentical