gh-128562: Fix tkinter widget instance name sometimes duplicated on inherited class #128604
Conversation
Xiaokang2022
commented
Jan 8, 2025
•
Xiaokang2022
commented
- Issue: tkinter widget instance name sometimes duplicated on inherited class #128562
Lib/tkinter/__init__.py
Outdated
| while cls.__module__ not in ("tkinter", "tkinter.ttk"): | ||
| cls = cls.__base__ |
There was a problem hiding this comment.
This works, but it's a little hacky, and probably going to lead to some unexpected results. The more ideal solution would be to actually check if the autogenerated name exists and then adjust accordingly (e.g. label2 could become label2_2).
There was a problem hiding this comment.
The more ideal solution would be to actually check if the autogenerated name exists and then adjust accordingly
However, this will cause the _name of the original normal widgets to be changed, which may lead to other incompatibilities. I'd prefer to just modify the name of the inherited class.
I now think that if the class name ends with a number, then we might be best able to put a special tag (This tag cannot appear in the class name) after its name. This may be a better solution. (e.g. label2 -> label2$)
What do you think of this solution?
There was a problem hiding this comment.
I now think that if the class name ends with a number, then we might be best able to put a special tag (This tag cannot appear in the class name) after its name.
The existing test code requires that the name must be a valid identifier. So there may not be a way to do that.
I'm not quite sure what to do right now is the best thing to do. :(
Sorry, I read it wrong, the existing test code requires that name is not a valid identifier.
ZeroIntensity
added
topic-tkinter
needs backport to 3.12
There was a problem hiding this comment.
$ has a special meaning in Tcl. It is better to avoid it. Use anything other, e.g. ! (- is already used in distinct scheme for Checkbutton).
Alternatively, we can change the scheme more radically -- using '%s!%d' instead of '!%s%d'. But this needs more thoughts.
There was a problem hiding this comment.
The test does not cover all code. Well, the existing test also does not cover all old code.
Add self.assertNotEqual(str(f), str(f2)) -- this will test the uniqueness of names for old scheme.
Now you need to create several instances of Button and Button2 (or Frame and Frame2, as there are already two Frame instances) and check that their names are unique, i.e. check the length of the set of their str() representations.
For now, the test is passed with unpatched code, except the last added line which tests an implementation detail.
Co-authored-by: Serhiy Storchaka <[email protected]>
Misc/NEWS.d/next/Library/2025-01-08-03-09-29.gh-issue-128562.Mlv-yO.rst
Outdated
Show resolved
Hide resolved
|
Thanks @Xiaokang2022 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
…-128604) There were possible conflicts if the widget class name ends with a digit. (cherry picked from commit da8825e) Co-authored-by: Zhikang Yan <[email protected]>
|
GH-128791 is a backport of this pull request to the 3.13 branch. |
…-128604) There were possible conflicts if the widget class name ends with a digit. (cherry picked from commit da8825e) Co-authored-by: Zhikang Yan <[email protected]>
|
GH-128792 is a backport of this pull request to the 3.12 branch. |
…) (GH-128791) There were possible conflicts if the widget class name ends with a digit. (cherry picked from commit da8825e) Co-authored-by: Zhikang Yan <[email protected]>
…) (GH-128792) There were possible conflicts if the widget class name ends with a digit. (cherry picked from commit da8825e) Co-authored-by: Zhikang Yan <[email protected]>
