Roundup Robot added the comment:
New changeset 65e5f28801e1 by R David Murray in branch '3.2':
#15510: clarify textwrap's handling of whitespace, and add confirming tests.
http://hg.python.org/cpython/rev/65e5f28801e1
New changeset 0e9ad455355b by R David Murray in branch 'default':
Merge
R. David Murray added the comment:
Thanks, Chris.
--
resolution: - fixed
stage: patch review - committed/rejected
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
Chris Jerdonek added the comment:
I responded to David's comments on the review tool. Later today I will update
the patch in response to his comments (accommodating all of his suggestions)
along with a couple other changes.
--
___
Python tracker
Raymond Hettinger added the comment:
FWIW, I agree that the existing behavior should not be changed. Most likely, a
change would break some code that is currently working, and there would be
little to no gain.
--
nosy: +rhettinger
___
Python
Chris Jerdonek added the comment:
Attaching an updated patch as promised in my previous comment.
Note that I removed two edge test cases pertaining to leading whitespace. I
would rather discuss those cases as part of a different issue to avoid making
this thread even longer (and it is off
Chris Jerdonek added the comment:
Thanks a lot for the very thorough review, David. I should be able to update
the patch and respond to a couple of your points later today or tomorrow at the
latest.
--
___
Python tracker rep...@bugs.python.org
R. David Murray added the comment:
Added some review comments on issue-15510-4.patch.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
___
Chris Jerdonek added the comment:
Attaching an updated patch that improves the organization of the new test cases
(test ordering, test names, and test comments, etc).
--
Added file: http://bugs.python.org/file27043/issue-15510-4.patch
___
Python
Chris Jerdonek added the comment:
Here is a patch that documents and adds tests for the existing behavior (i.e.
keeping the current behavior the same).
I also expanded the patch slightly to cover related edge cases that involve the
interplay between whitespace, empty lines, and indenting
Ethan Furman added the comment:
Chris Jerdonek wrote:
Here is an example on a paragraph with line breaks between paragraphs:
s/paragraph/text/
def wrap_paras(text, width=70, **kwargs):
... lines = [line for para in text.splitlines()
... for line in wrap(para, width,
R. David Murray added the comment:
Also you will note that the return of the empty list for an empty line is
exactly what you want for wrapping multiple line-break-delimited paragraphs.
Consider:
doc = a para\nanother para\n\na third, but with an extra blank line
between\n
for line in
Greg Ward added the comment:
Random comments from original author of textwrap.py, in case anyone cares at
this point...
* This is a pretty obscure edge case, and I admit that it never occurred to me
when writing the code. (If it had, I would have tested it!)
* One can debate endlessly about
Chris Jerdonek added the comment:
Thanks for weighing in, Greg!
At least for me, this edge case was important because it determines how the
canonical advice or recipe for handling multiple paragraphs behaves when the
input string has line breaks in between paragraphs. That advice, of course,
R. David Murray added the comment:
FTR I agree with Antoine that returning the empty list is the more logical
behavior here. Wrap is turning a string into a list of lines...if there is no
content, the list of lines *should* be empty, IMO. That is what I would
expect, so for me the empty
Jesús Cea Avión added the comment:
I think this is a bug. The question to ponder is backwards compatibility,
specially if this is going to be backported to 2.7/3.2.
Chris, could you possibly ask for opinions in python-dev and/or python-list?
--
stage: patch review -
Changes by Jesús Cea Avión j...@jcea.es:
--
stage: - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
___
___
Python-bugs-list
Antoine Pitrou added the comment:
I think this is a bug.
Do you have any argument?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
___
Ethan Furman added the comment:
The documentation says, Returns a list of output lines; an empty list is not
a list of lines.
--
nosy: +stoneleaf
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
Ethan Furman added the comment:
Not sure I would worry about fixing it in 2.7, although I don't have strong
feelings about that.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
___
Antoine Pitrou added the comment:
an empty list is not a list of lines
Really?
.splitlines()
[]
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
___
Ethan Furman added the comment:
Antoine Pitrou wrote:
Antoine Pitrou added the comment:
an empty list is not a list of lines
Really?
.splitlines()
[]
Really.
-- ''.split('\n')
['']
--
___
Python tracker rep...@bugs.python.org
Ethan Furman added the comment:
Ethan Furman wrote:
Antoine Pitrou added the comment:
.splitlines()
[]
-- ''.split('\n')
['']
I see the docs have been fixed in 3 to explain the not present last
empty line.
However, sure this is still not correct?
-- wrap(' ')
[]
So if you have
Antoine Pitrou added the comment:
Really.
-- ''.split('\n')
['']
You claimed that an empty list is not a list of lines. I countered that
splitlines(), which *by definition* returns a list of lines, can return
an empty list, therefore textwrap.wrap() is not exotic in its behaviour.
Whether
Ethan Furman added the comment:
Antoine Pitrou wrote:
Antoine Pitrou added the comment:
Really.
-- ''.split('\n')
['']
You claimed that an empty list is not a list of lines. I countered that
splitlines(), which *by definition* returns a list of lines, can return
an empty list,
Antoine Pitrou added the comment:
For an empty string, sure -- for a string with nothing but white space,
no:
-- wrap(' ')
[]
That's because wrap() suppresses extra whitespace by default. Once extra
whitespace is suppressed, you are left with an empty text, meaning an empty
list of
Chris Jerdonek added the comment:
That's because wrap() suppresses extra whitespace by default.
But the documentation for drop_whitespace clearly states that, after wrapping,
leading whitespace in the first line is always preserved, though.
Once extra whitespace is suppressed, you are left
Antoine Pitrou added the comment:
That's because wrap() suppresses extra whitespace by default.
But the documentation for drop_whitespace clearly states that, after
wrapping, leading whitespace in the first line is always preserved,
though.
Ok, then it's a bit fuzzy. That whitespace is
Chris Jerdonek added the comment:
wrapping, leading whitespace in the first line is always preserved,
though.
Ok, then it's a bit fuzzy. That whitespace is as much trailing as
leading, after all :)
That's why the word always is there. :)
I'm not sure I see the relevance. strip() returns
Antoine Pitrou added the comment:
Le samedi 04 août 2012 à 00:05 +, Chris Jerdonek a écrit :
Chris Jerdonek added the comment:
wrapping, leading whitespace in the first line is always preserved,
though.
Ok, then it's a bit fuzzy. That whitespace is as much trailing as
leading,
Antoine Pitrou added the comment:
Le samedi 04 août 2012 à 00:32 +, Chris Jerdonek a écrit :
But I feel this criterion was not applied to issue 1859. wrap()'s
behavior on newlines is broken to the point that multi-paragraph input
is acknowledged as not working. Additionally, because of
Chris Jerdonek added the comment:
As far as these changes don't fix obvious bugs, no, they shouldn't.
People certainly rely on the current behaviour, and they will start
getting extraneous newlines if you change it (because they will call
'\n'.join(...)).
That line of reasoning is acceptable
Chris Jerdonek added the comment:
Ok, I probably read the issue too quickly. Feel free to ignore my
comment then :-)
Thanks. I will prepare another patch for this issue with documentation and
test cases of existing behavior. The discussion can of course continue if
anyone would like to
Chris Jerdonek added the comment:
I verified that this patch can be applied to 2.7 and that those tests pass as
well.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15510
___
Antoine Pitrou added the comment:
Uh, how is this a bug? An empty text doesn't contain lines at all, so returning
an empty list of lines sounds right.
Furthermore, by fixing this, you may break existing software.
--
nosy: +pitrou
___
Python tracker
Chris Jerdonek added the comment:
Here are additional test cases impacted by this issue:
wrap( )
[]
wrap(\n\n\n)
[]
wrap(\n\n\n, replace_whitespace=False)
[]
wrap( \n\n, replace_whitespace=False)
[]
--
___
Python tracker
Jesús Cea Avión added the comment:
I think this is a bug.
Could you possibly write a patch for 2.7, 3.2 and 3.3?. With a test :-)
I am worried about changing a behaviour that somebody is depending of. Opinions?
Adding a couple of persons I think that could have an opinion about this.
Chris Jerdonek added the comment:
Could you possibly write a patch for 2.7, 3.2 and 3.3?. With a test :-)
I would be happy to write a patch with tests (I think there may be a few edge
cases we would want to test here). Should be ready within the next one or two
days.
--
Chris Jerdonek added the comment:
Attaching patch with test cases.
I added 6 new tests:
*test_empty_string
test_whitespace_trailing
*test_drop_whitespace__all_whitespace
*test_initial_indent__empty_string
test_subsequent_indent__trailing_whitespace
test_subsequent_indent__long_indent
The
Chris Jerdonek added the comment:
Actually, here is a slightly more benign version of the patch.
This patch makes the change in the wrap() method and leaves _wrap_chunks()
alone.
This is less intrusive because it doesn't change the behavior of
_wrap_chunks(), which some people might be
New submission from Chris Jerdonek:
While working on issue 1859, I found that textwrap.wrap() returns an empty list
when passed the empty string:
from textwrap import wrap
wrap('')
[]
as opposed to a list containing the empty string which is what I expected--
['']
I originally accepted
40 matches
Mail list logo
