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.

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

gh-103685: Fix tkinter.Menu.index() for Tk 8.7 #103686

Merged
merged 7 commits into from
Apr 24, 2023

Conversation

chrstphrchvz
Copy link
Contributor

@chrstphrchvz chrstphrchvz commented Apr 22, 2023

@terryjreedy
Copy link
Member

Christopher: Add blurb with something like "Make tkinter.Menu.index('none') continue to work in tk 8.7."

@serhiy-storchaka Do we consider tkinter failures due to tcl/tk changes to be bugs, such that fixes should be backported? I am thinking so and plan to merge and backport unless you say something.

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
@serhiy-storchaka
Copy link
Member

I agree that it should be backported.

@chrstphrchvz Do existing tests cover this case? Or a new test is needed?

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.11 only security fixes label Apr 23, 2023
@chrstphrchvz
Copy link
Contributor Author

@chrstphrchvz Do existing tests cover this case? Or a new test is needed?

I did not notice the error in any existing test.test_tkinter tests, so I have added a test for this. The error does prevent IDLE from launching (at least on Aqua), though.

This PR is probably one of the smaller and easier to understand changes needed for Tk 8.7, but in practice it is likely not useful without first addressing e.g. #103194, so presumably there would be more to backport.

@terryjreedy terryjreedy marked this pull request as ready for review April 23, 2023 20:49
@terryjreedy
Copy link
Member

Properly testing tkinter, etc, with 8.7 requires a buildbot that installs the latest 8.7, which we don't have yet. I have no idea whether the linux x simulator can simulate 8.7 yet.

Again, please add a blurb. Perhaps "Make tkinter.Menu.index('none') work in tcl/tk versions being developed." is slightly better that what i wrote above.

@terryjreedy terryjreedy merged commit f0ed293 into python:main Apr 24, 2023
@miss-islington
Copy link
Contributor

Thanks @chrstphrchvz for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 24, 2023
---------

(cherry picked from commit f0ed293)

Co-authored-by: Christopher Chavez <chrischavez@gmx.us>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
@bedevere-bot
Copy link

GH-103734 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Apr 24, 2023
@chrstphrchvz chrstphrchvz deleted the patch-103685 branch April 24, 2023 01:34
terryjreedy added a commit that referenced this pull request Apr 24, 2023
…03734)

gh-103685: Fix tkinter.Menu.index() for Tk 8.7 (GH-103686)

---------

(cherry picked from commit f0ed293)

Co-authored-by: Christopher Chavez <chrischavez@gmx.us>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir topic-tkinter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants