-
-
Notifications
You must be signed in to change notification settings - Fork 32k
Tkinter: Don't stringify callback arguments #66410
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
Comments
Currently Tkinter supports two modes:
But arguments of a callback are always converted to str, even when wantobjects=1. This can lost information ("42" is not distinguished from 42 or (42,)) and may be less efficient. Unfortunately we can't just change Tkinter to pass callback arguments as Python objects, this is backward incompatible change. Proposed patch introduces third mode wantobjects=2. It is the same as wantobjects=1, but callbacks now received . Default mode is still wantobjects=1, it can be changed in future. But in Tkinter applications (IDLE, Turtledemo, Pynche) we can use wantobjects=2 right now. The only problem left is documenting it. The wantobjects parameter is not documented at all except an entry in "What's New in Python 2.3". Does anyone want to document this feature? |
I think I requested elsewhere that public _tkinter/tkinter names be documented. What you wrote already seems clear enough. "Default mode is still wantobjects=1," The patch appears to change it to 2 in tkinter/init.py. "/* Create argument list (argv1, ..., argvN) */ /arg/args/ is definitely less confusing. Add /list/tuple/ to old and new functions. One thing slightly puzzles me: the current PythonCmd is used in a call to Tcl_CreateCommand. (Since the latter is not in _tkinter, I presume it is 'imported' in one of the includes.) The new PythonObjCmd is passed to Tcl_CreateObjCommand. Does that already exist, just waiting for us to add PythonObjCmd? I near as I can tell, the only differences between PythonCmd and PythonOjbCmd are /argv/objv/ and the following.
PyObject *s = unicodeFromTclString(argv[i + 1]);
PyObject *s = FromObj(data->self, objv[i + 1]); I think I would make the name substitution either in both or neither. It would be nice to reuse the rest of the code. |
This is for development only. You can apply the patch and test how new mode
All Tcl_* functions are Tcl C API functions. Old functions work with strings
I'm not sure that I understand all you mean, but in updated patch the common |
Updated patch. Added NEWS and What's New entries. |
There is no review button for the new patch (don't know why), so: in the News and What's New entries, change "if call" to "if one calls" and "or set" to "or sets". More importantly, question your use of 'callback' in the entries. To me, a callback is a function that is passed to a recipient to be called *by the recipient* at some later time. For tkinter, there are command callbacks, which get no arguments, validatecommand callbacks, which get string arguments, event callbacks, which get a tkinter event object as argument, and after callbacks, which get as arguments the Python objects given to the after call. In all cases, there is no visible issue of passing non-strings as strings to the callback. The following duplicates the testfunc and interp.call in test_tcl.py as closely as I know how, but with a different result. import tkinter as tk
root = tk.Tk()
def test(*args):
for arg in args:
print(type(arg), '', repr(arg))
root.after(1, test, True, 1, '1')
root.mainloop() To me, the revised test does not involve a callback. The test registers a name for the python function so it can be passes by name to Tk().call, as required by the .call signature. So I would replace 'callback' with 'Python function registered with tk'. Registered functions can, of course, be used as callback if they have the proper signature. ConfigDialog registers an 'is_int' function so it can be passed to an Entry as a validatecommand callback. The patch does not affect registered validate commands because they receive string args. After grepping idlelib for "register(", I believe this is the only function IDLE registers with tk (two of its classes have non-tk .register methods). Hence, IDLE should not be affected by the patch and seems not as far as I tested. I can't properly review the _tkinter patch as it requires too much knowledge of tcl/tk interfaces. |
Thank you Terry for your review. Here is updated patch fith fixed wording as you suggested. Hope it will be available to review on Rietveld. The original purpose of this patch was to help fixing some IDLE "crashes". Percolator is registered in text widgets for highlighting pasted text. If you paste invalid string containing a lone surrogate on Windows (clipboard uses UTF-8), it first converted by Tcl to char*, and then Tkinter should convert it to Python string for passing to registered Python function, but fails. Since the information is lost during conversion in Tcl, we can't use say "surrogatepass" error handler fo representing non-well-formed data. With this patch we can decode just from Tcl unicode string. |
Review works. "to Python function" should be either "to a Python function" or "to Python functions". Preventing crashes is a justifying use case. Percolator registers its 'insert' and 'delete' methods with a WidgetRedirector. It then patches them into to text instance, thereby inducing tk to call its replacements without their being registered with tk. Can you add a test case based on this? I tried the following, based on what Redirector does, without and with the patch and got the same result each time.
import tkinter as tk
root = tk.Tk()
text = tk.Text(root)
s = '\ud800'
text.insert('1.0', s)
print(text.get('1.0'))
def insert(index, s):
Text.insert(text, index, s)
text.insert('1.1', s)
print(text.get('1.1'))
root.clipboard_append(s)
text.insert('1.2', text.clipboard_get())
print(text.get('1.2'))
### output
�
�
Traceback (most recent call last):
File "F:\Python\mypy\tem2.py", line 12, in <module>
text.insert('1.2', text.clipboard_get())
File "F:\Python\dev\36\lib\tkinter\__init__.py", line 730, in clipboard_get
return self.tk.call(('clipboard', 'get') + self._options(kw))
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xed in position 0: invalid continuation byte Can the above be changed to fail, then succeed? |
Callbacks registered in the tkinter module now take arguments as various Python objects (int, float, bytes, tuple), not just str. To restore the previous behavior set tkinter module global wantobject to 1 before creating the Tk object or call the wantobject() method of the Tk object with argument 1. Calling it with argument 2 restores the current default behavior.
PR #98592 is simpler than the initial patches. The What's New entry and the NEWS entry also were rewritten, because the behavior is now changed by default. Example in #66410 (comment) now fails on |
Callbacks registered in the tkinter module now take arguments as various Python objects (int, float, bytes, tuple), not just str. To restore the previous behavior set tkinter module global wantobject to 1 before creating the Tk object or call the wantobject() method of the Tk object with argument 1. Calling it with argument 2 restores the current default behavior.
…nGH-98592) Callbacks registered in the tkinter module now take arguments as various Python objects (int, float, bytes, tuple), not just str. To restore the previous behavior set tkinter module global wantobject to 1 before creating the Tk object or call the wantobject() method of the Tk object with argument 1. Calling it with argument 2 restores the current default behavior.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: