Repository navigation
tk busy command #72684
Copy link
Copy link
Closed
Labels
3.13only security fixesonly security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytopic-tkintertype-featureA feature request or enhancementA feature request or enhancement
Description
| BPO | 28498 |
|---|---|
| Nosy | @ned-deily, @serhiy-storchaka, @csabella |
| Files |
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:
assignee = 'https://github.com/serhiy-storchaka'
closed_at = None
created_at = <Date 2016-10-21.17:03:43.066>
labels = ['expert-tkinter', '3.8', 'type-feature', 'library']
title = 'tk busy command'
updated_at = <Date 2018-02-22.01:06:47.241>
user = 'https://bugs.python.org/tkinter'bugs.python.org fields:
activity = <Date 2018-02-22.01:06:47.241>
actor = 'ned.deily'
assignee = 'serhiy.storchaka'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Tkinter']
creation = <Date 2016-10-21.17:03:43.066>
creator = 'tkinter'
dependencies = []
files = ['45190', '45193', '45199', '45200', '45230', '45231', '45277', '45337']
hgrepos = []
issue_num = 28498
keywords = ['patch']
message_count = 43.0
messages = ['279140', '279168', '279171', '279196', '279210', '279216', '279223', '279226', '279232', '279244', '279246', '279247', '279252', '279262', '279272', '279274', '279277', '279280', '279282', '279283', '279286', '279287', '279289', '279290', '279291', '279292', '279293', '279294', '279295', '279300', '279302', '279322', '279323', '279505', '279506', '279507', '279510', '279675', '279726', '279738', '279746', '279990', '312521']
nosy_count = 6.0
nosy_names = ['klappnase', 'ned.deily', 'serhiy.storchaka', 'tkinter', 'GNJ', 'cheryl.sabella']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue28498'
versions = ['Python 3.8']Linked PRs
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
3.13only security fixesonly security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytopic-tkintertype-featureA feature request or enhancementA feature request or enhancement
Activity
tkinter commentedon Oct 21, 2016
tcl tk 8.6.6 has a new busy command. The new tkinter library doesn't provide an interface for this command.
https://www.tcl.tk/man/tcl/TkCmd/busy.htm
The solution is to add to the class Misc of tkinter these methods:
klappnase commentedon Oct 21, 2016
Funny thing, just today I came across the same issue.
So far I came up with an a little bit refined version of the OP's suggestions. Until now I have tested this with Python-2.7.9 and tk-8.6.0 on debian Jessie, I think the same code should work for Python-3 , too.
The busy_current() function here always reports only the root window (the same when called from within wish, apparently a bug in this early version of Tk-8.6) so I was not able to figure out how to use and test the "pattern" option.
klappnase commentedon Oct 21, 2016
Oops, just made a quick test with Python3, and found that I had missed that types.StringType is no longer present. So to be compatible with Python3 the busy_configure() func ought to look like:
tkinter commentedon Oct 22, 2016
Maybe it's better to add also these methods:
busy = tk_busy
busy_cget = tk_busy_cget
busy_configure = tk_busy_configure
busy_current = tk_busy_current
busy_forget = tk_busy_forget
busy_hold = tk_busy_hold
busy_status = tk_busy_status
Many other methods in tkinter module follow the same pattern. For example:
klappnase commentedon Oct 22, 2016
Ok, I investigated this a little further. First I noticed another bug with the code from my first post, the "self._w" must be omitted from the call to busy_current(), so the func should look like:
I did then some more testing, the code now seems to work flawlessly both with Python-3.5.2 on windows 10 and with Python 2.7 on debian Jessie. So I finally prepared a patch (against the __init__.py file from the 3.5.2 windows installer). Following Miguel's suggestion I also added aliases prefixed with tk_ to the various busy_... methods.
Since I am not familiar with Python's "official" test mechanisms, for now I wrote a simple script that does a number of calls to the tk_busy_... functions to test if everything works as expected:
try:
import Tkinter
except:
import tkinter as Tkinter
#Tkinter.wantobjects = False
serhiy-storchaka commentedon Oct 22, 2016
Could you provide a patch against the default branch? See https://docs.python.org/devguide/ for help. The patch should include not just changes to the tkinter module, but tests for new methods (add new class in Lib/tkinter/test/test_tkinter/test_widgets.py). Unfortunately there is no appropriate place for documenting this feature in the module documentation, but new methods should have docstrings. Would be nice to add an entry in Doc/whatsnew/3.7.rst.
tkinter commentedon Oct 22, 2016
Thanks klappnase for your collaboration.
I dont understand this function:
def busy_status(self):
'''Returns the busy status of this window.
If the window presently can not receive user interactions,
True is returned, otherwise False.'''
return((self.tk.getboolean(self.tk.call(
'tk', 'busy', 'status', self._w)) and True) or False)
This pattern is not used in other functions that make use of self.tk.getboolean. These functions simply returns the value of self.tk.getboolean directly.
The code of your function busy_configure() is very similar to Misc._configure(). I think that you are duplicating code. Other functions related to configuration like pack_configure() and place_configure() simply use self._options(). For example:
def pack_configure(self, cnf={}, **kw):
self.tk.call(
('pack', 'configure', self._w)
+ self._options(cnf, kw))
I think that my proposal can do the job well. It follows the same pattern than the other functions:
def tk_busy_configure(self, cnf=None, **kw):
self.tk.call(('tk', 'busy', 'configure', self._w) + self._options(cnf, kw))
But I am not totally sure whether it's better to call directly Misc._configure or Misc._options in this situation.
Also if we follow the naming convention used in tkinter, it seems that we have to define first tk_busy_configure and then make this assignation:
busy_configure = tk_busy_configure
tkinter commentedon Oct 22, 2016
Misc._configure is only used when the first Tcl command is the name of the widget.
Very probably my proposal for tk_busy_configure is a better candidate because it follows the conventions used in tkinter (it's similar to pack_configure and place_configure):
def tk_busy_configure(self, cnf=None, **kw):
self.tk.call(('tk', 'busy', 'configure', self._w) + self._options(cnf, kw))
klappnase commentedon Oct 22, 2016
@miguel
About tk_busy_configure():
I strongly suggest that my version of tk_busy_configure() is better, since it behaves exactly like the usual configure() methods in Tkinter (which includes e.g. Canvas.itemconfigure() or Text.tag_configure()), i.e. you can not only change the values of the configuration options, but also query available options and their default values. I think *this* follows the usual Tkinter conventions.
As you pointed out in your last post, I could not use _configure() directly for this, but had to "duplicate" the code with a slight modification, because of the order of arguments for tk_busy_configure().
The reason that this construct is not used for pack_configure() is simply that widget.pack_configure() does not return anything if called without arguments (in fact it appears to be the very same as widget.pack()).
About busy_status():
the construct I use here is to work around the broken _getboolean() which does not always return True or False, but often returns None instead of False (if wantobjects==True). Therefore I used (unlike most Tkinter methods) tkapp.getboolean() directly, however quite a while ago I noticed that this was broken, too, and sometimes returned 0 or 1 instead of True or False. However I admit that I did not test right now if this is still the case, in fact I just copy'n'pasted it from code I already use and that works perfectly well (BTW, I suggested the same method to fix these issue with _getboolean() here some time ago, and of course I agree that it would be preferrable to fix _getboolean() and possibly tkapp.getboolean() in the first place).
@serhiy
Thanks for the link, I did not find the source tree on the new python web site (I admit I did not look too hard :) .
I now added a patch against the default branch'es __init__.py . Doc strings are (still :) included. I also changed the function names according to Miguel's suggestion.
I'll see if I find the time to add a proper test class one of these days (you guys make it really hard for outsiders to help out ;)
tkinter commentedon Oct 23, 2016
Hi klappnase,
you are right with the function tk_busy_configure(). Maybe there is a little bit of code duplicated. I think that it's better to directly change the Tkinter function _configure() to make it more general:
def _configure(self, cmd, cnf, kw):
"""Internal function."""
if kw:
cnf = _cnfmerge((cnf, kw))
elif cnf:
cnf = _cnfmerge(cnf)
if cnf is None:
return self._getconfigure(cmd)
if isinstance(cnf, str):
return self._getconfigure1(cmd + ('-'+cnf,))
self.tk.call(cmd + self._options(cnf))
I think that it's interesting to do this change because the function is more general and maybe can be used again in the future for new features in Tcl Tk. This way, we avoid code duplication (Dont Repeat Yourself is one of the philosophes of Python). This is the new version of tk_busy_configure using the mentioned change:
def tk_busy_configure(self, cnf=None, **kw):
return self._configure(('tk', 'busy', 'configure', self._w), cnf, kw)
It's more concise.
I disagree with tk_busy_status(). It's better to be more predictable an return the same than the other methods that uses getboolean(). If there is a bug, it's better to solve that bug. Could you please guive me an code example that replicates the bug?
A priori , I can't appreciate any bug in this function. If there is some bug, it's in another place of the C code.
32 remaining items
klappnase commentedon Oct 26, 2016
This is something entirely different, since the commands you list here in tk do not accept keyword arguments at all.
tkinter commentedon Oct 29, 2016
Why dont we do the job well from the beginning and refactor _configure() and adapt other dependent code? It's not so complicated to change the dependent code. Many people around the world use Tkinter.
serhiy-storchaka commentedon Oct 30, 2016
Added a number of style comments on Rietveld. I believe klappnase could address them, but since there are too little time to beta3 and I have a weak hope to push this in 3.6, I addressed them myself. Rewritten docstrings and test.
Ned, is it good to add this feature in 3.6? The patch just adds a couple of new methods, it doesn't affect existing programs.
ned-deily commentedon Oct 30, 2016
With ActiveState 8.6.4.1 (the most recent version available) on macOS:
======================================================================
ERROR: test_tk_busy (tkinter.test.test_tkinter.test_misc.MiscTest)
----------------------------------------------------------------------
serhiy-storchaka commentedon Oct 30, 2016
Ah, yes, this feature is not supported on OSX/Aqua.
serhiy-storchaka commentedon Nov 3, 2016
Updated patch skips tests with the cursor option on OSX/Aqua.
csabella commentedon Feb 22, 2018
There was a lot of work put into this patch, but I don't think it's been committed yet. Are any of the original authors interested in converting this to a Github pull request on the master branch? Thanks!
pythongh-72684: Tkinter: provide interface for "tk busy" subcommands
gh-72684: Tkinter: provide interface for "tk busy" subcommands (GH-10…