classification
Title: tkinter: change API of non-functioning peer_create, does not instantiate Text
Type: enhancement Stage: patch review
Components: Tkinter Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ghoul, gpolo, markroseman, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2013-05-09 14:36 by ghoul, last changed 2015-08-28 20:07 by markroseman.

Files
File name Uploaded Description Edit
pythonlevelpeers.py ghoul, 2013-05-09 14:36 Python script to make peers python objects
issue17945.diff terry.reedy, 2013-05-10 20:06 Polo's patch review
peerfix.diff ghoul, 2013-05-13 20:06
Messages (12)
msg188778 - (view) Author: Gregory HOULDSWORTH (ghoul) Date: 2013-05-09 14:36
Python version: 3.3.0 32bit
OS: Windows 7 Service Pack 1 64bit

The peer_create method of the Text class, introduced in changeset
<71041c0dedd5> in response to issue <2843> creates a tk text widget
but does not instantiate Text. Because they don't exist as Python
objects, we have to use tk.call() to manipulate them. This also breaks
the nametowidget method used with Text's peer_names, again because
these widgets don't exist in Python.

Following the inheritance of Text in /Lib/tkinter/__init__.py:2039
Text > Widget > BaseWidget, where the actual widget creation occurs
with a call to self.tk.call:

self.tk.call(
            (widgetName, self._w) + extra + self._options(cnf))

We cannot smuggle in the tk command '.myoriginaltext peer create' as
a tuple in place of the widgetName ('text') because we end up with the
malformed argument:

((.myoriginaltext, 'peer', 'create'), .peer.path ...)

The attached file is my second attempt, using subclassing.
Lot of code duplication, ugly and not thoroughly tested: I'm hoping a
more elegant solution can be found.
msg188782 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2013-05-09 16:02
Uh.. well observed.

It sounds like you are the first actual user of peer_create. Now I wish Tk
had done this in a different way: when creating a text widget, specificy
that it is a peer of some other text widget via an option ("-peer w" for
example).

If possible I would like to keep the functionality through the method
"peer_create" (and as a minor detail, newPathName should be None by default
-- it is easy to pick some name for the peer).
msg188784 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2013-05-09 16:43
Here is a quick patch for it: http://pastebin.com/m1XQBGqU (I forgot my
password for the tracker, and leaving home right now).

Does it work for you ?
msg188787 - (view) Author: Gregory HOULDSWORTH (ghoul) Date: 2013-05-09 18:20
Splendid, it works and is indeed far more elegant.
Well done there, thanks.
msg188872 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-05-10 20:06
(This issue only applies to 3.3+ because the method is new in 3.3.)

Gregory: 'works for me' means that the issue can be closed without a patch because Python actually 'works' for the example presented. I presume you mean that the tkinter works *after* you apply the patch and therefore consider it a possible fix to be applied. On that assumption, I downloaded it and will upload it here.

Also, if you specifically write issue2843 or #2843, the tracker makes a link to the issue.
msg188908 - (view) Author: Gregory HOULDSWORTH (ghoul) Date: 2013-05-11 12:00
Noted: I assumed 'works for me' meant user approval of proposed fix,
pending 'official' sanction.
Didn't catch the BNF-like syntax for issue linking, hence the literal
<>'s in my original post.

Thank you for clarifying those.
msg188932 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2013-05-11 16:34
If someone decides to commit this, please check that the name of the widget
in Tcl is always adequate.
msg189174 - (view) Author: Gregory HOULDSWORTH (ghoul) Date: 2013-05-13 20:06
The Text instance created by the last patch has the same parent -in
the Python w hierarchy- as the "model" widget regardless of the actual
parent implied by the given pathname. Further, pathname is really a tk
level construct: in Python this hierarchy is expressed the w's master
and name. Attached is a proposed fix; peer_create now has the
signature (self, master=None, cnf={}, **kw)
The optional name of the peer is passed as an item in cnf.
(note that the behaviour of peers is actually correct in the previous
incarnation, only the children property is faulty and you have to go
down to tk names to attach the text to an arbitrary parent)

There is also an issue with subclassing of Text, to override the
standard event handling for example. Should the peer be an instance of
that subclass, or should this be a 'default' that may be overriden?
msg248956 - (view) Author: Mark Roseman (markroseman) * Date: 2015-08-21 14:40
Just came across this one (Terry I'm sure can figure out why).

I'm sympathetic with Guilherme's wish that Tk had done this one differently, as an option to specify an existing text widget peer when creating a text widget, rather than having the original spawn a new widget. It probably has something to do with specifying a "-peer" option that you couldn't later change via "configure".

As Gregory's peerfix.diff indicated, the original Python API for this command is wrong, or at least at odds with all the rest of Tkinter. The one required argument should indeed be the parent (master) for the new peer, and not it's widget path name. The call should return the new Text widget object.

Given that nobody can be using the (unpatched) call as it is not functional, I suggest this be changed, without regard to backward compatibility concerns.

The other issue that Gregory raises is the peer being an instance of a Text subclass, e.g. _textpeer, rather than Text itself. While I'm not certain, I suspect this could likely be an issue in practice.

If it's somehow possible to create it as an instance of Text, that would definitely be preferable. I haven't closely examined BaseWidget et al., but I wonder if a hack along these lines might be possible:

 1. Create a new Text widget under the parent (call it 'dummy')
 2. Generate a new widget name (or used a passed in 'name' cnf parameter as usual)  to be used for the peer text widget
 3. Call the underlying 'peer create' with our generated name to create a new text widget on the Tcl side
 4. Modify dummy._w to use the generated name
 5. Replace the original parent.children[dummy._name] entry with one for the newly generated name
 6. Call into Tcl to destroy the dummy Text widget

It didn't look to me like there would be more housekeeping that needed doing to effectively point a widget object at a different widget.

The overhead of temporarily creating the extra Text widget should be negligible in practice.

If this seems like a feasible approach I can try to code it up.
msg248966 - (view) Author: Mark Roseman (markroseman) * Date: 2015-08-21 17:51
Did a quick check... this approach seems to work. Only change was in step 4 should set both ._w and ._name for the widget object
msg249293 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-08-28 19:30
I think it would be better to add a way to create the Text object (or may be any widget object) for existing Tk widget. Similar problem exists for cloned menus (issue734176).
msg249295 - (view) Author: Mark Roseman (markroseman) * Date: 2015-08-28 20:07
I'm not against generality, though don't think there are many places this actually comes up (the cloned menus shouldn't be affecting more than a couple people these days, and aren't the result of an explicit API call). 

For this particular call (peer_create) I just think the caller should be passing in the parent window as in other Tkinter widget creation commands, rather than the full widget path. On the exact implementation of the call I'm open to whatever works!
History
Date User Action Args
2015-08-28 20:07:37markrosemansetmessages: + msg249295
2015-08-28 19:30:38serhiy.storchakasetmessages: + msg249293
2015-08-21 20:40:39terry.reedysetnosy: + serhiy.storchaka
title: tkinter/Python 3.3.0: peer_create doesn't instantiate Text -> tkinter: change API of non-functioning peer_create, does not instantiate Text
type: behavior -> enhancement

versions: + Python 3.6, - Python 3.3, Python 3.4
2015-08-21 17:51:06markrosemansetmessages: + msg248966
2015-08-21 14:40:38markrosemansetnosy: + markroseman
messages: + msg248956
2013-05-13 20:06:08ghoulsetfiles: + peerfix.diff

messages: + msg189174
2013-05-11 16:34:23gpolosetmessages: + msg188932
2013-05-11 12:00:19ghoulsetmessages: + msg188908
2013-05-10 20:06:49terry.reedysetfiles: + issue17945.diff

versions: + Python 3.4
keywords: + patch
nosy: + terry.reedy

messages: + msg188872
resolution: works for me ->
stage: patch review
2013-05-09 18:20:16ghoulsetresolution: works for me
messages: + msg188787
2013-05-09 16:43:53gpolosetmessages: + msg188784
2013-05-09 16:02:49gpolosetmessages: + msg188782
2013-05-09 14:36:22ghoulcreate