gh-144207: Syntax highlighting (theming support) for dis module

[AbduazizZiyodov]

About

Implemented in similar fashion (e.g unittest, difflib etc.) by defining ThemeSection and registered theme on Theme class.

Basic highlighting shown in forum(green for all opcodes), but in this PR: one color maps to group of opcodes (e.g. blue for binary opcodes) -- gives better context and it is determined via color_by_opname method.

I have trouble with tests, now dis is defaulting to colored output and 2 tests were failing. I had to set environment variable: NO_COLOR=1... I think, there might be better ways of doing this, one is to define force_no_color flag(keyword only) to dis.dis function which I'm not certain. So, I would like to elaborate with you on that.

EDIT: Any ideas/additions on color mapping is encouraged :)

Sample screenshots

(see #144208 (comment) for the latest)

In dark mode

In light mode

No color

Thanks.

• Issue: Syntax highlighting(theming support) for dis module #144207

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[StanFromIreland]

Please add a NEWS entry and update What's New.

[StanFromIreland]

Please add tests for the colouration.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[picnixz]

Please add a What's News entry and tests. I personally do not like this by default since I use grep/cut on the dis output so I would prefer a CLI option to enable colors. EDIT: not a concern anymore

[picnixz]

I am a bit confused with END_FOR and END_ASYNC_FOR being colored differently but I do not remember the exact effect of the latter.

[AbduazizZiyodov]

I've read that END_FOR is equivalent(or alias I would say) to POP_TOP:

Removes the top-of-stack item. Equivalent to POP_TOP. Used to clean up at the end of loops, hence the name.

and it is specified under "General instructions" section while END_ASYNC_FOR in "Coroutine opcodes" (I generalized this into "control flow" opcodes).

Almost all opcodes are dealing with stack, but END_ASYNC_FOR is putting little more effort than END_FOR which is just stack.pop(), that's why I thought END_ASYNC_FOR is different than END_FOR.

That's my understanding.

We might elaborate our discussion on categories in your next comment too.

[picnixz]

How were those categories determined? are they determined already like that in dis.rst?

[AbduazizZiyodov]

are they determined already like that in dis.rst?

Almost, I first grouped according to dis.rst then re-categorized them according to my understanding (how these opcodes relate, semantically) -- which might need some refinement too.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[hugovk]

I personally do not like this by default since I use grep/cut on the dis output

Not to worry, it can stay on by default because colour is automatically disabled in pipes:

[picnixz]

Oh! great then. I was worried about this. Do we plan to have colors for JSON? (or maybe we already do?) or for symtable? (the symtable cli is very rudimentary and I sometimes want a better one).

There are few modules who output formatted code (AFAIR, dis, symtable, json or even ast) so I wondered whether those would also be colorized. And if we eventually plan to add colors wherever we can (it should be a separate DPO thread/gh issue)

[hugovk]

json, yes: https://docs.python.org/3/whatsnew/3.14.html#json. symtable, no[t yet].

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[sobolevn]

LGTM, I did not check that colorizations logic, because I am not an expert in it.

[picnixz]

I emitted some reservations in https://discuss.python.org/t/syntax-highlighting-for-dis-module/105833/14 about the explosion of colors. I would prefer that we have alternate colorization in terms of blocks like godbolt

[hugovk]

Where are we up to with this one? There's just over a couple of weeks before the 3.15 feature freeze.

@picnixz Is the current state a blocker for you?

[picnixz]

I have commented on the DPO thread about my concerns for the overusage of colors and suggested zebra lines per section.

[hugovk]

OK, thanks!

@AbduazizZiyodov Do you know what to do next? Do you have an idea of the colour scheme to use?

[AbduazizZiyodov]

OK, thanks!

@AbduazizZiyodov Do you know what to do next? Do you have an idea of the colour scheme to use?

I have some idea, the screenshot I've shared on discourse forum + coloring exception tables (e.g. magenta, red). Any suggestions are welcome - current color scheme(the way it "rendered") looks bit sharp to me.

[github-actions]

This PR is stale because it has been open for 90 days with no activity.

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #34127044 | 📁 Comparing c3378fb against main (a7bb524)

  🔍 Preview build  

3 files changed

± library/asyncio-task.html
± whatsnew/3.16.html
± whatsnew/changelog.html

[StanFromIreland]

I think adding an additional option for the block highlight is a little messy. Indeed it's currently not ideal, another option would be to use ESC[7m to invert, but that'll be quite a bright highlight. We could detect the user's background and adjust appropriately, but that's also messy.

Ultimately, while the background is nice, I don't consider it a deal-breaker.

[AbduazizZiyodov]

I think adding an additional option for the block highlight is a little messy.

Exactly, and my intention to introduce this here was for sake of experimenting (PoC) since PR is WIP/Draft.

Indeed it's currently not ideal, another option would be to use ESC[7m to invert, but that'll be quite a bright highlight. We could detect the user's background and adjust appropriately, but that's also messy.

+1

Ultimately, while the background is nice, I don't consider it a deal-breaker.

So, we(I) can keep it as ast/tokenize-way like plain highlighting ? I'll have a look on highlighting "logic" again (i.e. what to highlight, how etc.)

[StanFromIreland]

So, we(I) can keep it as ast/tokenize-way like plain highlighting ?

That seems fine to me.

[AbduazizZiyodov]

Marking PR as ready now.

• I think it is final version (for me). And now argument detail is "rendered" as an italic (-1 colors)
• News entry was added, moved "whatsnew" section into 3.16.rst (i hope its appropriate)
• Tests are here too

Screenshots (ast -> tokenize -> dis):

Dark color scheme:

Light color scheme:

On bigger scope:

Dark color scheme

Light color scheme

---

CC: @picnixz @StanFromIreland @hugovk @sobolevn

Thanks.

[cdce8p]

Not sure how others feel about it but I'm not sure this actually improves readability. To me the colors in this example feel much more distracting than anything else. I believe it came up in the discuss thread as well that compiler explorer/godbolt is generally a reasonable / good output. While I agree that banding might be too much for the terminal, maybe the color choices can at least be adopted. Most notably that the instructions are usually just one color. Only jump targets appear to be highlighted. That would reduce the visual noice and make it easier to read IMO. For reference: https://discuss.python.org/t/syntax-highlighting-for-dis-module/105833/20

Side note for the argument details: Not sure italics are helpful here. I'd prefer unformatted text over that. If the intention just was to reduce the number of different colors, I do think the compiler explorer style for highlighting these would work as well. Especially if the number of colors for instructions is reduced.

--
Edit: Linking the screenshot seems to be broken. I'm referring to the one from your last post: #144208 (comment), the "bigger scope" example.