classification
Title: Add textwrap.indent() as counterpart to textwrap.dedent()
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: amaury.forgeotdarc, brandj, chris.jerdonek, dontknow, elsdoerfer, ezberch, georg.brandl, ncoghlan, python-dev, r.david.murray, rutsky
Priority: normal Keywords: patch

Created on 2012-01-25 03:37 by ncoghlan, last changed 2012-06-12 15:00 by chris.jerdonek. This issue is now closed.

Files
File name Uploaded Description Edit
indent.patch ezberch, 2012-01-31 00:03 patch implementing textwrap.indent, plus tests and documentation review
Messages (22)
msg151932 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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 (brandj) 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) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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 msg151945. 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) Date: 2012-06-12 15:00
Great. Looks good!
History
Date User Action Args
2012-06-12 15:00:38chris.jerdoneksetmessages: + msg162681
2012-06-11 13:31:24ncoghlansetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-06-11 13:16:45ncoghlansetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg162615

stage: resolved -> patch review
2012-06-11 13:08:34python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg162613

resolution: fixed
stage: patch review -> resolved
2012-06-11 10:53:37ncoghlansetassignee: ncoghlan
stage: needs patch -> patch review
2012-05-29 22:37:14ncoghlansetmessages: + msg161917
2012-05-29 15:26:59r.david.murraysetmessages: + msg161893
2012-05-29 14:51:54chris.jerdoneksetmessages: + msg161890
2012-05-29 14:21:29r.david.murraysetnosy: + r.david.murray
messages: + msg161885
2012-05-29 13:24:14ncoghlansetmessages: + msg161879
2012-05-13 05:12:35chris.jerdoneksetmessages: + msg160507
2012-05-13 04:16:46chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg160505
2012-04-22 19:51:09elsdoerfersetnosy: + elsdoerfer
2012-02-19 12:45:35rutskysetnosy: + rutsky
2012-02-02 22:07:20ezberchsetmessages: + msg152482
2012-02-02 21:25:20georg.brandlsetmessages: + msg152477
2012-01-31 00:03:47ezberchsetfiles: + indent.patch

nosy: + ezberch
messages: + msg152363

keywords: + patch
2012-01-30 21:02:59ncoghlansetmessages: + msg152346
2012-01-30 19:44:32dontknowsetnosy: + dontknow
messages: + msg152343
2012-01-29 14:29:57georg.brandlsetmessages: + msg152231
2012-01-29 14:29:43georg.brandlsetnosy: + georg.brandl
messages: + msg152230
2012-01-25 14:26:05brandjsetnosy: + brandj
messages: + msg151946
2012-01-25 13:59:03ncoghlansetmessages: + msg151945
2012-01-25 11:16:09amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg151940
2012-01-25 10:17:34ncoghlansetmessages: + msg151938
2012-01-25 03:37:15ncoghlancreate