Repository navigation
Idle: increase max calltip width #64537
Description
| BPO | 20338 |
|---|---|
| Nosy | @terryjreedy, @skrah, @serhiy-storchaka |
| 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/terryjreedy'
closed_at = <Date 2014-01-27.01:37:06.993>
created_at = <Date 2014-01-21.22:41:42.381>
labels = ['expert-IDLE', 'type-feature']
title = 'Idle: increase max calltip width'
updated_at = <Date 2019-03-25.08:06:32.750>
user = 'https://github.com/terryjreedy'bugs.python.org fields:
activity = <Date 2019-03-25.08:06:32.750>
actor = 'terry.reedy'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2014-01-27.01:37:06.993>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2014-01-21.22:41:42.381>
creator = 'terry.reedy'
dependencies = []
files = ['33632', '33676', '33720']
hgrepos = []
issue_num = 20338
keywords = ['patch']
message_count = 22.0
messages = ['208716', '208717', '208718', '208735', '208783', '208832', '208833', '208839', '208893', '208900', '208901', '209041', '209042', '209288', '209292', '209372', '209374', '209391', '209523', '209545', '209610', '209627']
nosy_count = 4.0
nosy_names = ['terry.reedy', 'skrah', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20338'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']
Activity
terryjreedy commentedon Jan 21, 2014
Idle calltips are currently limited to 75 (signature) or 70 (doc line) chars. This is insufficient for some stdlib signatures. Example:
The truncated tip displayed is "(self, host, port=None, strict=<object object at 0x03125148>, timeout=<object obj ..."
Given that Idle is a graphics application, not a 80-char text console application, there is no obvious reason for the particular restriction other that esthetics. The tip is in a popup window that is separate from the shell or editor window where '(' was typed. On my widescreen monitor, there is room for, say, 200 chars. If I move the underlying window to the far right, so there is not enough room for even the truncated "HTTPConnection(" tip, Windows just truncates it further, as with any other window.
Serhiy, if you have a *nix machine, can you verify that there is no problem with that either?
The limits appear in multiple places in CallTipWindow.showtip and CallTips.get_argspec. Since get_argspec truncates a docstring line to less that showtip allows, there is no ' ...' added and showtip only affects inspect signatures. Get_argspec could leave length enforcement, whatever we decide it should be, to showtip.
I think the first thing to do is replace the multiple hard-coded numbers with one named constant.
terryjreedy commentedon Jan 21, 2014
Meant to mention that wrapping is a possible alternative to truncation, although a limit of say, 200, would make either alternative rare.
terryjreedy commentedon Jan 21, 2014
On further thought, I think the best place to truncate lines is in get_argspec, after being split from the docstring and before rejoined.
terryjreedy commentedon Jan 22, 2014
I deleted truncation code in CallTipWindow.py, all three versions, but messed up issue number.
2.7 http://hg.python.org/cpython/rev/c28e07377b03
changeset: 88630:c28e07377b03
3.3 (same) http://hg.python.org/cpython/rev/1b89fd73c625
changeset: 88631:1b89fd73c625
Issue still open for changing CallTips._MAX_COLS.
serhiy-storchaka commentedon Jan 22, 2014
There is no problem on Linux.
I think wrapping will be more preferable. 200 columns looks to wide for tips.
Perhaps in 3.4 we can use textwrap.wrap() with the max_lines argument. And left simple truncating in earlier versions.
terryjreedy commentedon Jan 22, 2014
How about we extend the limit from 79 to say 120, to accommodate people who use longer docstring lines, and only wrap the signature line, when present, like the one for HTTPConnection, as the signature is the important information to include.
serhiy-storchaka commentedon Jan 22, 2014
Yes, I think we can extend the limit (until implement wrapping). On my narrow monitor there is a room only for 120 character tip (when it starts right from left side of the screen), so I prefer to wrap long lines. In any case too long lines are not comfortable to read.
serhiy-storchaka commentedon Jan 22, 2014
Here is a patch (for 3.4) which wraps long lines. Now you can display a tip for textwrap.TextWrapper.
terryjreedy commentedon Jan 23, 2014
I want to keep the code as simple and fast as possible, and not worry much about rare oddball or PEP-8-violating cases. I want to only do what is needed for the purpose of calltips, which is to remind one of the call sequence. Adding more detracts from that purpose. Tips are not intended to replace using the doc or help(f) to learn about f.
As it is, I am not happy about possibly including more than 1 or 2 lines of a docstring for Python-coded functions. I intend to revisit that when the dust settles with Clinic and inspect. That fact that another patch *will* be needed, almost certainly, is another reason to not do too much now.
Wrapping overly long docstrings should not needed. Even if the initial _MAX_COL slice of the first line of the docstring of a Python-coded function fails to say enough about the return value, there is help(f) to get full details.
I reread PEP-8 and, contrary to what I (mis) remembered, it says that docstrings should remain limited to 72 (or 79) chars *even if* a project increases the limit for *code* lines up to say, 100. The reason for this is precisely because other software, like help, Idle, and others, read, process, and display docstrings. If docstrings violate this, I think they should be changed, and not accomadated (except as suggested below).
So I want to add, before the docstring fetch, only this:
dict is definitely an oddball and I do not want to include it in a test at this time. I do not mind suggesting help(dict), though the current docstring is incomplete. Here is the real signature and description, which I verified from the examples below. The multiple example lines should follow a condensed summary.
<start current docstring with specifics>
'''
First, when wrapping a signature, the 'words' after the first few typically have the form 'name=value'. They will average much longer than the average English word (4, I think). The average wrapped line length will be _MAX_COLS - half the average 'word' length. For example (one which should become a test):
(width=70, initial_indent='', subsequent_indent='', expand_tabs=True,
replace_whitespace=True, fix_sentence_endings=False, break_long_words=True,
drop_whitespace=True, break_on_hyphens=True, tabsize=8, *, max_lines=None,
placeholder=' [...]')
The lengths, including indents, are 69, 78, 77, and 24, all <= 79. Very few signature lines, wrapped or not, will fall in [80-85].
Second, PEP-8 allows initial summary lines to be 79 chars, as they are not "flowing long blocks of text". (I verified this with Guido.) Some docstrings in the stdlib 'cheat' a few chars, and 85 will accommodate those without ever being excessively long.
serhiy-storchaka commentedon Jan 23, 2014
Well, do this.
I think we should add several test functions with too long signatures and/or docstrings.
serhiy-storchaka commentedon Jan 23, 2014
And note that currently there is a bug in doc.split('\n', 5)[:_MAX_LINES]. An argument of split() should be _MAX_LINES.
terryjreedy commentedon Jan 24, 2014
I changed 5 to _MAX_LINES.
Other than trivial duplication, I think each test example should be added for a reason, because there is something different about it that might conceivably cause a failure. For example, there should be a case with more than _MAX_LINES in the docstring. But I think one is enough to make sure we split with maxsplit. And I think one text wrap is enough.
The many 't#' test functions were needed for 2.x because it has 12 lines of complicated code that were replaced here by 1.
argspec = inspect.formatargspec(*inspect.getfullargspec(fob))
For 3.x, we would not need as many tests if the inspect methods were adequately tested and debugged. However, they were not fixed to handle keyword only args until 3.4 and they are still in flux.
The attached patch will need adjustment for 2.7 (I believe) and 3.4 (because *,... is handled better). I plan to do so when fresher.
serhiy-storchaka commentedon Jan 24, 2014
textwrap.TextWrapper in 3.3 doesn't have '*' and following parameters.
terryjreedy commentedon Jan 26, 2014
I removed the incorrect * comment.
Attached is a 2.7 port of the 3.3 patch. It turns out that textwrap.TextWrap in 2.7 is an old-style class. Such did not work previously because they do not have .__call__ and failed the hasattr() check, now a try:except check. In 2.7, callable(old) == True, so there must be an explicit type check. I added one in the patch.
serhiy-storchaka commentedon Jan 26, 2014
LGTM. But I prefer larger indent.
Perhaps we need explicit tests for constructors of old- and new-style classes, with __new__, with __init__, and without __new__ and __init__.
python-dev commentedon Jan 27, 2014
New changeset d960c4cabb2b by Terry Jan Reedy in branch '2.7':
Issue bpo-20338: Increase allowed tip width slightly and wrap long signagure lines.
http://hg.python.org/cpython/rev/d960c4cabb2b
New changeset 7dcc19308a7b by Terry Jan Reedy in branch '3.3':
Issue bpo-20338: Increase allowed tip width slightly and wrap long signagure lines.
http://hg.python.org/cpython/rev/7dcc19308a7b
New changeset f7061219303c by Terry Jan Reedy in branch 'default':
Issue bpo-20338: Increase allowed tip width slightly and wrap long signagure lines.
http://hg.python.org/cpython/rev/f7061219303c
terryjreedy commentedon Jan 27, 2014
I increased indent to 4 spaces and made it easier to change.
I believe old-styles classes do not have .__new__, so test of TextWrapper will ensure that they work.
Inspect.signature should have correct logic to handle .__new__, with or without .__init__. (I presume that if both are present, they must be consistent, so .__init__ should be used.) You might want to check both behavior and test file for .signature.
Idle gets .signature upgrades for free when we switch, which I hope can be in a few months at worst. Programmer use of .__new__ without .__init__ is rare enough (extremely so) that I prefer to wait. As it is, switching will already mean removing some of the current logic.
terryjreedy commentedon Jan 27, 2014
Actually, all new-style classes .__new__ and .__init__ from object if not over-ridden. The calltip code depends on the .__init__ inheritance.
skrah commentedon Jan 28, 2014
I think test_idle is failing on many build slaves following this
commit.
terryjreedy commentedon Jan 28, 2014
On 1/28/2014 4:48 AM, Stefan Krah wrote:
The two failures I saw on the first 4 3.x bots with Idle failures are
the result of changes in the list docstrings. I presume these are
related to use of Argument Clinic. I knew and documented that such
failure was a possibility.
On my Win 7 machine
At some point, I will switch to using inspect.signature for Idle
calltips and simplify the Idle code. I will also simplify the tests and
make them more robust.
I will re-compile and adjust the tests by the end of the day.
Terry
terryjreedy commentedon Jan 29, 2014
In rev88792, UTC 13:00 Larry fixed this mismatch by patching
test_calltips.py to match the new string.
and fixed fixed the first-line deletion somewhere in changes to about 30
other files.
Until I know that there is another test somewhere that docstrings are
not somehow mangled, I will leave some tests here that they are as expected.
I recompiled but will do something else instead.
Terry
skrah commentedon Jan 29, 2014
Terry J. Reedy <report@bugs.python.org> wrote:
Ah, so it was a Derby accident. Let's hope things stabilize soon.