gh-79951: IDLE - Convert menudefs to dictionary

[csabella]

PoC to convert menudefs to a dictionary to make indexing the correct menu easier.

• Issue: IDLE: python -m idlelib fails on master on Mac OS 10.10.4 #79951

[terryjreedy]

My previous comment, based on insufficient information, assumed that you were doing a total conversion to a dict of dicts. Only converting the top level list of tuples of key and sublists makes this much more feasible, and it should make testing a bit easier and mistakes less likely. Passing on Appveyer is very encouraging.

I believe the trailing whitespace in zzdummy is before the deleted ')'. Will fix. Wait to fit the conflict until I fix the fix (after I submit this).

After opening a new issue, change the bpo number in the initial comment (3 places) as well as the title.

I am pretty sure the extension menu processing code can be adjusted to handle both old and new formats. A test_zzdummy should try both. This is needed first, perhaps as a separate issue.

A possible solution to the 'application' issue is to add 'application':None as the first dict item and then ignore or delete it if not Carbon Aqua. Tal's solution is not bad since it only copies references to the value lists.

[csabella]

I think there's an easy solution for extensions. In fill_menus, add after the check for None:

if isinstance(menudefs, list):
    menudefs = dict(menudefs)

Should I modify the PR? I didn't want to change it if you were in the middle of pushing a change.

[terryjreedy]

That looks plausible, and I am done. Go ahead with revisions.

[csabella]

I've added the change for menudefs and I also added the fill_menus test from #3669, with some changes for the new menudefs type.

[taleinat]

Have you tested this with existing extensions? IIRC they usually modify the menudefs rather than override them. If so, the backwards compatibility approach used here won't work. Rather, we'll need to still expose menudefs as a list of tuples, but perhaps have an alternate internal representation as a dict.

Alternatively, and more simply, drop backwards compatibility, and just have this be a dict from now on. Extensions can add a version check and appropriate code for versions where this has been introduced. This is simple enough that any IDLE extensions actually in use that would be used with such new versions of IDLE would be easily upgraded. This is what I'm leaning towards.

[taleinat]

Another option: We could also keep the existing "list of tuples" representation, and just add a few helper functions for access by name rather than by index. This would be 100% backwards compatible, make IDLE's code more readable, and reduce the likelihood of indexing bugs like #79951. So perhaps the best of both worlds.

[taleinat]

To summarize, the 3 options I've lined out are:

1. Convert to dicts, keeping external "list of tuples"-like interface.
2. Break backwards compatibility and just use a dict.
3. Stay with the current "list of tuples" representation; add a few helper functions for by-name access.

[terryjreedy]

The intent of the issue is a) to make idlelib code a little clearer and robust and b) to facilitate translations. I will comment more on the latter elsewhere another time. I do not intend to abandon past extensions, at least not with this change. So I expect to revise the code that merges extension menu defs after I otherwise like the patch.

The dummy extension, zzdummy, exists to test things like this. There is no automated test (yet). To use it, the menudefs have to be uncommented. I think I commented them out to avoid having inactive menu entries or something. Then, does IDLE start, is the menu altered, and does it work.