Repository navigation
IDLE Output Window 's goto fails when path has spaces #49809
Description
| BPO | 5559 |
|---|---|
| Nosy | @terryjreedy, @kbkaiser, @bitdancer |
| 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/kbkaiser'
closed_at = <Date 2011-05-13.03:42:57.562>
created_at = <Date 2009-03-25.03:22:34.067>
labels = ['expert-IDLE', 'type-bug']
title = "IDLE Output Window 's goto fails when path has spaces"
updated_at = <Date 2011-05-13.03:42:57.561>
user = 'https://bugs.python.org/ccanepa'bugs.python.org fields:
activity = <Date 2011-05-13.03:42:57.561>
actor = 'kbk'
assignee = 'kbk'
closed = True
closed_date = <Date 2011-05-13.03:42:57.562>
closer = 'kbk'
components = ['IDLE']
creation = <Date 2009-03-25.03:22:34.067>
creator = 'ccanepa'
dependencies = []
files = ['13848']
hgrepos = []
issue_num = 5559
keywords = ['26backport']
message_count = 12.0
messages = ['84142', '84158', '85663', '86628', '86629', '86634', '86642', '86995', '87035', '104448', '104535', '135887']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'kbk', 'gpolo', 'r.david.murray', 'ccanepa']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue5559'
versions = ['Python 2.6', 'Python 3.0', 'Python 3.1', 'Python 2.7']
Activity
ccanepa commentedon Mar 25, 2009
in windows XP, python 2.6.1, 2.6 , python 2.4
result]
\line' pop up
If the path in the target line has spaces, it will popup a window with
title 'No special line' and message 'the line you point at doenst look
like a valid file name followed by a line number'
posible fix:
in idlelib/OutputWindow.py
replace the literal
r'([^\s]+):\s*(\d+):'
with
r'([^\t\n\r\f\v]+):\s*(\d+):'
fair warning: seems to work in windows XP, dont know about other OSes
gpolo commentedon Mar 25, 2009
I see this occurring everywhere, but the proposed solution may not be
enough. I can still create a directory with a tab on it and it will fail
too.
The proper solution (in my head) would involve changing the interaction
of GrepDialog with OutupWindow, but then OutputWindow wouldn't really be
an OutputWindow. But I believe the better way to fix this right now is
to define a more correct regex, and test it :)
ccanepa commentedon Apr 6, 2009
On a second look:
the code in OutputWindow.py for the 'goto' action looks for a match
with the first regex in
file_line_pats = [
r'file "([^"])", line (\d+)',
r'([^\s]+)\((\d+)\)',
r'([^\s]+):\s(\d+):',
]
and it assumes the first group gives a valid filename, the second a
line number.
the potential target lines produced by GrepDialog.py are writen by:
sys.stdout.write("%s: %s: %s\n" % (fn, lineno, line))
where:
fn :a valid filename ( because an open(fn) was issued before), not
guaranted an abspath
lineno : unsigned int
line: a text line in an arbitrary file, but mostly an *.py
Clearly the 3rd regex is the only one that can hit the line produced by
GrepDialog, and clearly will fail in any OS that allows spaces in a
valid path: the regex breaks the first group at the first whitespace.
The tentative fix propossed in the initial post was a minimal change
that allow spaces in the path, but was very ugly and not comunicates a
clear intention.
I like more:
r'(?:\s*)(.+):\s+(\d+):'
wich discards leading whitespace ( feature unused by GrepDialog but
probably handy in pyShell, wich subclasses OutputWindow )
It is a better regex that the original in IDLE, meaning it would hit
more positives, but can fail sometimes:
Supose the GrepDialog found a hit at line 111 in the file c:\foo.py ,
with the text
a = 1 # see 24: 32: 1
The line sent to OutputWindow would be
c:\foo.py: 111: a = 1 # see 24: 32: 1
and the regex will capture the groups:
filename = "c:\foo.py: 111: a = 1 # see 24"
linenum = "32"
The first group fails to capture the filename.
I can live with such special case failures, but anyway:
In windows, changing the regex to break the first group at the
first ': ' would fix the thing ( ': ' cant happen in a fn that pass open
(fn) )
How about other OSes ?
------
kbkaiser commentedon Apr 26, 2009
Added a regex to handle win paths w/spaces.
A regex match may not be the file desired.
Try all the patterns until a valid file is found.
r71995.
Let me know if you can find a failing corner case.
port to 3 and -maint.
gpolo commentedon Apr 26, 2009
Wouldn't it be better to actually write tests for it ? Also, the issue's
title is a bit misleading, it is not only spaces that causes troubles.
Isn't fixing how GrepDialog <-> OutputWindow works an acceptable
solution ? Instead of just writing the output in the OutputWindow, can't
we store the results generated by GrepDialog so OutputWindow can access
it without even needing to retry some regexes till it works (or not) ?
kbkaiser commentedon Apr 27, 2009
I'd welcome a test, please provide one. I actually wrote a small test
code so I could understand the problem, but didn't include it.
The 'grep dialog' sends its output to an OutputWindow labelled *Output*
as a series of lines. This is entirely separate from the "go to file/
line" code and the path issue raised in this bug. The line being
queried may also be a PyShell traceback.
When you right click on a line (any line in an OutputWindow), the
string is passed to OutputWindow.goto_file_line(), which uses
_file_line_helper() to try to find a valid file. The problem was the
regex couldn't handle spaces (or tabs) correctly, and also the code gave
up if the first match didn't result in a valid file. Typically, it was
trying a path which started with the first character after the last
embedded space. A match, but invalid. To get to the correct pattern,
the code must not give up.
We are just trying patterns on a string w/o going back to the *Output*
window. That's reasonable, because, for example, the traceback line
looks quite different from the grep dialog output and needs a different
regex. It's certainly not inefficient, because the human right-clicked a
single line and won't wake up for 500 ms, at least :-)
We have a simple problem to fix, and don't want use it as an excuse to
tear up a lot of code which is working well.
Actually, I ran into this myself this morning while hacking trunk IDLE
on Windows from a DOS box using python 2.6, and I had the problem half
fixed before I saw this bug! Amazing it took this long to raise it, the
code here hasn't changed for many, many years.
The regex I added is specifically for Win paths. It ltrims, then takes
everything up to the first ':'. The trick is to be non-greedy.
Do you have a case the corrected code doesn't handle?
gpolo commentedon Apr 27, 2009
Create a file that starts with a space and search for something that
returns it, for example.
kbkaiser commentedon May 3, 2009
r72227.
How's your test code coming? A relative Win filename with leading
spaces should be found even when there's a file of same name but no
spaces.
ccanepa commentedon May 3, 2009
On Sat, May 2, 2009 at 11:31 PM, Kurt B. Kaiser <report@bugs.python.org>wrote:
Sorry for the delay, Kurt.
Test with rev 72227 , ok.
test cases:
(one space betwwen a and b , one before second tmp, one before first x)
Searching 'hello' in d:\tmp\*.tmp ...
d:\tmp\ xx.tmp: 1: hello # see woot.py:24 :24
d:\tmp\a b\ tmp\ xx.tmp: 1: hello # see woot.py:24 :24
d:\tmp\a b\ xx.tmp: 1: hello # see woot.py:24 :24
Found 3 hits.
( all three opens the correct file)
Same changing the target lines to stress the regex:
d:\tmp\ xx.tmp: 1: hello # see woot.py: 24:24
d:\tmp\a b\ tmp\ xx.tmp: 1: hello # see woot.py: 24:24
d:\tmp\a b\ xx.tmp: 1: hello # see woot.py: 24:24
Found 3 hits.
( all three opens the correct file)
Non absolute path:
xx.py: 1: hello # see woot.py: 24:24
xx.py: 1: hello # see woot.py: 24:24
Found 2 hits.
( both opens the correct file)
others; not listed: ok.
Thanks for taking care Kurt.
terryjreedy commentedon Apr 28, 2010
Resolution should only be set when an issue is closed. So should this be closed? or unfixed and the versions updated?
bitdancer commentedon Apr 29, 2010
I think the issue is still open waiting for a unit test. On the other hand, how likely is it that someone is going to write one at this point? If someone doesn't speak up soon I think you can close it.
kbkaiser commentedon May 13, 2011
Backported to 2.6 4Oct09 56387:490190cb4a57