-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
Tkinter widget.unbind(sequence, funcid) unbind all bindings #75666
Comments
I am using python 3.6.2 with tk 8.6.7 in Linux and when I call widget.unbind(<Sequence>, funcid), it unbinds all bindings for <Sequence>, while I would expect it to unbind only funcid. Here is an example reproducing the problem: import tkinter as tk
root = tk.Tk()
def cmd1(event):
print('1')
def cmd2(event):
print('2')
def unbind1():
l.unbind('<Motion>', b1)
def unbind2():
l.unbind('<Motion>', b2)
l = tk.Label(root, text='Hover')
b1 = l.bind('<Motion>', cmd1)
b2 = l.bind('<Motion>', cmd2, True)
l.pack()
tk.Button(root, text='Unbind 1', command=unbind1).pack()
tk.Button(root, text='Unbind 2', command=unbind2).pack()
root.mainloop() In this example, clicking on one of the buttons unbinds both bindings instead of only one. |
I have found a workaround to unbind a single binding (inspired by https://mail.python.org/pipermail/tkinter-discuss/2012-May/003151.html): def unbind(widget, seq, funcid):
bindings = {x.split()[1][3:]: x for x in widget.bind(seq).splitlines() if x.strip()}
try:
del bindings[funcid]
except KeyError:
raise tk.TclError('Binding "%s" not defined.' % funcid)
widget.bind(seq, '\n'.join(list(bindings.values()))) |
Tk do not provide a way of unbinding a specific command. It supports binding a script with replacing an existing binding, appending a script to an existing binding, and destroying an existing binding. Unbinding a specific command can be implemented in Python with a code similar to your example, but more complex due to taking to account different corner cases. Do you want to write a patch and open a pull request Juliette? |
The unbind docstring, "Unbind for this widget for event SEQUENCE the the docstring should be something like "Unbind for this widget the event SEQUENCE. If funcid is given, delete the command also" This part of the bind docstring, "Bind will return an identifier to allow deletion of the bound function with unbind without memory leak." implies that the unbind behavior is deliberate. Serhiy, are you proposing to add a new method that does what some people thought unbind would do? |
I don't think a new method is needed. We can make unbind() destroying all bindings if FUNCID is not given (note that registered commands are leaked in this case), and only the specified command if it is specified. We can add a boolean parameter (similar to the one in bind()) for restoring the old behavior if this is needed. |
I don't mind writing a patch and opening a pull request, however I don't know which corner cases I need to take into account. |
I propose the below fix:
|
Serhiy, if I understand your last message (msg302299), the docstring should read:
The main doc change is to specify what happens when funcid is not given while the code change in the patch is to no longer unbind everything when funcid *is* given. |
Serhiy, given that the code did not match the docstring and that we are changing the code to match the clarified docstring, should we consider this a bugfix, and, should we backport it? |
Will there be a change to the code? I would like to use the "single function unbind". |
The PR writer removed his cpython fork, and the PR was missing tests. So it must be rewritten and resubmitted by someone else. Since Serhiy is busy with other stuff, I may try to complete it some time, but I am not familiar with tkinter tests. There does seem to be enough material in the original report above to create one. |
How is it going? |
I am now working on other thing too, so anyone can try another patch. |
I am working on this. Other issues are that |
(cherry picked from commit 9bb202a) Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit 9bb202a) Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit 9bb202a) Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit 9bb202a) Co-authored-by: Serhiy Storchaka <[email protected]>
I have split all the changes into three parts.
Now, I am not sure what should be the target for the second change. Should it be backported? The current behavior only works as expected if there was only one callable bound to the sequence. If there was more than one, it can cause bad surprises. So it looks like a bug. What do you think, @terryjreedy? I am not going to backport the third change. In worst case the current code produces "leaks" of commands, creating more and more Tcl commands and references to Python callables until the widget be destroyed, if you constantly rebind the sequence. |
…"funcid" Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. Co-authored-by: GiovanniL <[email protected]>
To me, the 2nd fix is a bugfix that should be backported. Beyond the docstring, the Shipman doc says
Without checking back 20-30 years, I wonder if that ever was the case. |
In any case, the bind docstring says "Bind will return an identifier to allow deletion of the bound function with |
…d" (GH-111322) Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. Co-authored-by: GiovanniL <[email protected]>
…"funcid" (pythonGH-111322) Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. (cherry picked from commit cc7e45c) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: GiovanniL <[email protected]>
…"funcid" (pythonGH-111322) Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. (cherry picked from commit cc7e45c) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: GiovanniL <[email protected]>
… "funcid" (GH-111322) (GH-112801) Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. (cherry picked from commit cc7e45c) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: GiovanniL <[email protected]>
… "funcid" (GH-111322) (GH-112802) Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. (cherry picked from commit cc7e45c) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: GiovanniL <[email protected]>
After many years, thanks you are a savior |
Triage: can we close this issue now or is there more to be done? |
Not yet. |
…"funcid" (pythonGH-111322) Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. Co-authored-by: GiovanniL <[email protected]>
…"funcid" (pythonGH-111322) Previously, "widget.unbind(sequence, funcid)" destroyed the current binding for "sequence", leaving "sequence" unbound, and deleted the "funcid" command. Now it removes only "funcid" from the binding for "sequence", keeping other commands, and deletes the "funcid" command. It leaves "sequence" unbound only if "funcid" was the last bound command. Co-authored-by: GiovanniL <[email protected]>
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: