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.

[AbduazizZiyodov]

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) @picnixz

Indeed! symtable needs this enhancement as you mentioned, I'll work on this too after this PR gets merged, if anyone hasn't planned.

[StanFromIreland]

Please add tests.

[AbduazizZiyodov]

I'll focus on tests, and try suggested ideas from discussion forum too (probably reducing number of categories maybe).

[AbduazizZiyodov]

• Updated base theme, tried to apply suggestions from discussion thread. I would say, its more "load/pop" oriented now:

  Dark

  
  Light

  

• Added DisColoredTests test case. Another way would've been re-writing some(or all) of existing templates in "colored fashion" and test them separately - introduces a lot of redundancy (IMO).

  Example

   # existing template
   dis_f = """\
   %3d           RESUME                   0
   
   %3d           LOAD_GLOBAL              1 (print + NULL)
                 LOAD_FAST_BORROW         0 (a)
                 CALL                     1
                 POP_TOP
   
   %3d           LOAD_SMALL_INT           1
                 RETURN_VALUE
   """ % (_f.__code__.co_firstlineno,
          _f.__code__.co_firstlineno + 1,
          _f.__code__.co_firstlineno + 2)
   
   # to colored
   dis_f_colored = """\
   %3d           %sRESUME%s                   0
   %3d           %sLOAD_GLOBAL%s              1 %s(print + NULL)%s
                 %sLOAD_FAST_BORROW%s         0 %s(a)%s
                 %sCALL%s                     1
                 %sPOP_TOP%s
   %3d           %sLOAD_SMALL_INT%s           1
                 %sRETURN_VALUE%s
   """ % (_f.__code__.co_firstlineno, 
           theme.op_call_return, theme.reset,
          _f.__code__.co_firstlineno + 1,
          theme.op_load, theme.reset, theme.argument_detail, theme.reset,
          theme.op_load, theme.reset, theme.argument_detail, theme.reset,
          theme.op_call_return, theme.reset,
          theme.op_pop, theme.reset,
          _f.__code__.co_firstlineno + 2,
          theme.op_load, theme.reset,
          theme.op_call_return, theme.reset
   )

  I'm skeptic whether it would give enough value for effort spent. Let me know if you have thoughts on different method of testing there.

• Manually tested color customization option (as far as I know, its experimental) as mentioned in discussion forum:

  Default

  
  Customized

  
  Contents of PYTHONSTARTUP

   try:
       from dataclasses import replace
       from _colorize import ANSIColors, default_theme, set_theme
   except ImportError:
       pass
   else:
       print("Replacing theme ...")
       theme = default_theme.copy_with(
           dis=replace(
               default_theme.dis,
               label_bg=ANSIColors.BOLD_GREEN,
               label_fg=ANSIColors.BLACK,
               exception_label=ANSIColors.BOLD_GREEN,
               argument_detail=ANSIColors.BOLD_GREEN,
               op_load=ANSIColors.BOLD_GREEN,
               op_pop=ANSIColors.BOLD_YELLOW,
               op_call_return=ANSIColors.BOLD_MAGENTA,
               op_control_flow=ANSIColors.BOLD_CYAN,
               reset=ANSIColors.RESET,
           ),
       )
       set_theme(theme)

[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.