Add textwrap.indent() as counterpart to textwrap.dedent()

[ncoghlan]

BPO	13857
Nosy	@birkenfeld, @amauryfa, @ncoghlan, @bitdancer, @cjerdonek
Files	indent.patch: patch implementing textwrap.indent, plus tests and documentation

^{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/ncoghlan'
closed_at = <Date 2012-06-11.13:31:24.668>
created_at = <Date 2012-01-25.03:37:15.250>
labels = ['type-feature', 'library']
title = 'Add textwrap.indent() as counterpart to textwrap.dedent()'
updated_at = <Date 2012-06-12.15:00:38.252>
user = 'https://github.com/ncoghlan'

bugs.python.org fields:

activity = <Date 2012-06-12.15:00:38.252>
actor = 'chris.jerdonek'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2012-06-11.13:31:24.668>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2012-01-25.03:37:15.250>
creator = 'ncoghlan'
dependencies = []
files = ['24374']
hgrepos = []
issue_num = 13857
keywords = ['patch']
message_count = 22.0
messages = ['151932', '151938', '151940', '151945', '151946', '152230', '152231', '152343', '152346', '152363', '152477', '152482', '160505', '160507', '161879', '161885', '161890', '161893', '161917', '162613', '162615', '162681']
nosy_count = 11.0
nosy_names = ['georg.brandl', 'amaury.forgeotdarc', 'ncoghlan', 'r.david.murray', 'chris.jerdonek', 'elsdoerfer', 'rutsky', 'python-dev', 'dontknow', 'brandjon', 'ezberch']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue13857'
versions = ['Python 3.3']

[ncoghlan]

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)

[ncoghlan]

David Miller pointed out a shorter spelling:

    s.replace('\n', '\n' + (4 * ' '))

Still not particularly obvious to the reader (or writer), though.

[amauryfa]

If such a function is added, I'd like the option to not indent empty lines: trailing spaces are often not a good idea.

[ncoghlan]

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.

[brandjon]

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?

[birkenfeld]

BTW, the short spelling looks like it wouldn't indent the first line.

[birkenfeld]

Otherwise +1.

[dontknow]

Just wondering if someone is already working on this or am I free to supply a patch?

[ncoghlan]

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).

[ezberch]

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.

[birkenfeld]

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).

[ezberch]

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).

[cjerdonek]

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).

[cjerdonek]

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?

[ncoghlan]

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.

[bitdancer]

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.

[cjerdonek]

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', '', '']

[bitdancer]

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.

[ncoghlan]

I created bpo-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.

[python-dev]

New changeset 6f7afe25d681 by Nick Coghlan in branch 'default':
Close bpo-13857: Added textwrap.indent() function (initial patch by Ezra
http://hg.python.org/cpython/rev/6f7afe25d681

[ncoghlan]

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

[cjerdonek]

Great. Looks good!