| msg151932 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2012-01-25 03:37 |
| As far I am aware, the simplest way to indent a multi-line string is with the following snippet: '\n'.join((4 * ' ') + x for x in s.splitlines()) It would be a lot simpler and clearer if I could just write that as "textwrap.indent(s, 4 * ' ')". (i.e. indent would accept a prefix string to be inserted before each line in the supplied string, as in the original comprehension) |
|
|
| msg151938 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-01-25 10:17 |
| David Miller pointed out a shorter spelling: s.replace('\n', '\n' + (4 * ' ')) Still not particularly obvious to the reader (or writer), though. |
|
|
| msg151940 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2012-01-25 11:16 |
| If such a function is added, I'd like the option to not indent empty lines: trailing spaces are often not a good idea. |
|
|
| msg151945 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-01-25 13:59 |
| I'd actually suggest that as the default behaviour (and is a good argument in favour of a dedicated function in textwrap - both suggested alternatives will blithely add whitespace to otherwise empty lines). To handle the empty line requires either switching to an re.sub() based solution or adding a conditional expression: '\n'.join(((4 * ' ') + x if x else x) for x in s.splitlines()) I should probably also explicitly address the "why not textwrap.fill()?" alternative: because fill() does a lot more than simple indenting. |
|
|
| msg151946 - (view) |
Author: Jon Brandvein (brandjon) |
Date: 2012-01-25 14:26 |
| > If such a function is added, I'd like the option to not indent empty lines: trailing spaces are often not a good idea. From dedent's documentation, it wasn't immediately clear to me that it ignores blank lines when determining common whitespace. (In fact the comment in the example suggests otherwise.) Perhaps a note could be added to the documentation when this change is made? |
|
|
| msg152230 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2012-01-29 14:29 |
| BTW, the short spelling looks like it wouldn't indent the first line. |
|
|
| msg152231 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2012-01-29 14:29 |
| Otherwise +1. |
|
|
| msg152343 - (view) |
Author: John (dontknow) |
Date: 2012-01-30 19:44 |
| Just wondering if someone is already working on this or am I free to supply a patch? |
|
|
| msg152346 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-01-30 21:02 |
| Please go ahead! And Georg is right - the short spelling doesn't handle the first line correctly. It also suffers from the "trailing whitespace" problem that Amaury pointed out in my original version. The tests for the new function should check both those cases (i.e. don't indent blank lines, ensure the first line is correctly indented). |
|
|
| msg152363 - (view) |
Author: Ezra Berch (ezberch) |
Date: 2012-01-31 00:03 |
| I've created a patch using the conditional expression in . The one problem I found with it is that when the input string is terminated by a newline it removes that newline. I've added an optional third argument: a function which determines which lines are indented. If omitted, the default behavior is to indent non-empty lines. |
|
|
| msg152477 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2012-02-02 21:25 |
| IMO removing trailing newlines is not acceptable. You could use splitlines(keepends=True) to keep final newlines (but then the default function that determines lines to indent needs to ignore these newlines). |
|
|
| msg152482 - (view) |
Author: Ezra Berch (ezberch) |
Date: 2012-02-02 22:07 |
| Sorry, I guess I wasn't clear. The trailing-newlines issue was an issue with the conditional expression ncoghlan suggested. It's fixed in the patch I submitted (and covered by the tests). |
|
|
| msg160505 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-05-13 04:16 |
| I'd like to see this enhancement as well. It seems that not even a TextWrapper is capable of a simple indent (because TextWrapper methods operate on "paragraphs" rather than lines). |
|
|
| msg160507 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-05-13 05:12 |
| Should the function work for strings with non-Unix line endings? http://docs.python.org/dev/py3k/reference/lexical_analysis.html#physical-lines For example, should indent("abc\r\n", "") return the same string, and should "\r\n" get indented by default? |
|
|
| msg161879 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-05-29 13:24 |
| Added some review comments. I'm thinking the docs for str.splitlines() could really do with an update to say explicitly that a trailing newline *doesn't* append an empty string to the result. |
|
|
| msg161885 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-05-29 14:21 |
| Why would you expect it to? >>> 'a\nb'.splitlines() ['a', 'b'] >>> 'a\nb\n'.splitlines() ['a', 'b'] >>> 'a\nb\n\n'.splitlines() ['a', 'b', ''] That's exactly what I would intuitively expect, and I don't see how it could possibly do anything else. |
|
|
| msg161890 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-05-29 14:51 |
| Perhaps because that's what str.split() does: >>> "a\nb".split("\n") ['a', 'b'] >>> "a\nb\n".split("\n") ['a', 'b', ''] >>> "a\nb\n\n".split("\n") ['a', 'b', '', ''] |
|
|
| msg161893 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-05-29 15:26 |
| That's why it's a different function :) (Well, that and universal newline support). But I can see that explaining the difference between split and splitlines would be worthwhile. |
|
|
| msg161917 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-05-29 22:37 |
| I created #14957 to cover improving the str.splitlines docs. For this patch, I think Chris is right that it should be using str.splitlines(True) and joining on "''" instead of "'\n'" so that Windows line endings get preserved. |
|
|
| msg162613 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-06-11 13:08 |
| New changeset 6f7afe25d681 by Nick Coghlan in branch 'default': Close #13857: Added textwrap.indent() function (initial patch by Ezra http://hg.python.org/cpython/rev/6f7afe25d681 |
|
|
| msg162615 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-06-11 13:16 |
| Ezra (and anyone interested) may want to take a look at the checked in version to see some of the changes I made while preparing the patch for commit. - name changes and slight restructure as discussed on the review - splitlines() invocation changed as discussed above - doc examples changed to doctest style - tests reworked to use a parameterised style (taking the easy way out of just failing on the first broken case, since there aren't that many cases and the test is quick to run) - default predicate reworked to round trip with textwrap.dedent |
|
|
| msg162681 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-06-12 15:00 |
| Great. Looks good! |
|
|