tkinter: change API of non-functioning peer_create, does not instantiate Text

[ghoul]

BPO	17945
Nosy	@terryjreedy, @roseman, @serhiy-storchaka
Files	pythonlevelpeers.py: Python script to make peers python objects issue17945.diff: Polo's patch peerfix.diff

^{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 = None
closed_at = None
created_at = <Date 2013-05-09.14:36:22.353>
labels = ['type-feature', 'expert-tkinter']
title = 'tkinter: change API of non-functioning peer_create, does not instantiate Text'
updated_at = <Date 2015-08-28.20:07:37.285>
user = 'https://bugs.python.org/ghoul'

bugs.python.org fields:

activity = <Date 2015-08-28.20:07:37.285>
actor = 'markroseman'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Tkinter']
creation = <Date 2013-05-09.14:36:22.353>
creator = 'ghoul'
dependencies = []
files = ['30183', '30205', '30254']
hgrepos = []
issue_num = 17945
keywords = ['patch']
message_count = 12.0
messages = ['188778', '188782', '188784', '188787', '188872', '188908', '188932', '189174', '248956', '248966', '249293', '249295']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'gpolo', 'markroseman', 'serhiy.storchaka', 'ghoul']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17945'
versions = ['Python 3.6']

[ghoul]

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.

[gpolo]

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).

[gpolo]

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 ?

[ghoul]

Splendid, it works and is indeed far more elegant.
Well done there, thanks.

[terryjreedy]

(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 bpo-2843 or bpo-2843, the tracker makes a link to the issue.

[ghoul]

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.

[gpolo]

If someone decides to commit this, please check that the name of the widget
in Tcl is always adequate.

[ghoul]

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?

[roseman]

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.

[roseman]

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