Add flake8 plugin to check textwrap.dedent usage.

[kaos]

#17465 (comment) spurred me to add this flake8 lint rule. Not that it was an oversight in this case, but for anyone else not familiar with softwrap, this will help them find it rather than dedent.

It does not check test sources, so using dedent in tests are still perfectly valid.

[benjyw]

Why? What is bad about using dedent?

[benjyw]

Are we certain that softwrap emits the same as dedent in cases where that is what we'd want?

[kaos]

Why? What is bad about using dedent?

Dedent works well for code blocks, but doesn't reflow lines to adapt to the terminal width for text like messages etc.

Are we certain that softwrap emits the same as dedent in cases where that is what we'd want?

@thejcannon has written pretty extensive tests to ensure the formatting works in a predictable manner. The one case where it doesn't, is when you want hard line breaks. Example:

pants/src/python/pants/engine/target.py

Lines 2890 to 2924 in e9ae0a3

	def generate_file_based_overrides_field_help_message(
	generated_target_name: str, example: str
	) -> str:
	example = textwrap.dedent(example.lstrip("\n"))
	example = textwrap.indent(example, " " * 4)
	return "\n".join(
	[
	softwrap(
	"""
	Override the field values for generated `{generated_target_name}` targets.
	Expects a dictionary of relative file paths and globs to a dictionary for the
	overrides. You may either use a string for a single path / glob,
	or a string tuple for multiple paths / globs. Each override is a dictionary of
	field names to the overridden value.
	For example:
	"""
	),
	example,
	softwrap(
	f"""
	File paths and globs are relative to the BUILD file's directory. Every overridden file is
	validated to belong to this target's `sources` field.
	If you'd like to override a field's value for every `{generated_target_name}` target
	generated by this target, change the field directly on this target rather than using the
	`overrides` field.
	You can specify the same file name in multiple keys, so long as you don't override the
	same field more than one time for the file.
	"""
	),
	],
	)

Tests for softwrap:

pants/src/python/pants/util/strutil_test.py

Lines 172 to 361 in e9ae0a3

	def test_softwrap_multiline() -> None:
	assert (
	softwrap("The version of the prior release, e.g. `2.0.0.dev0` or `2.0.0rc1`.")
	== "The version of the prior release, e.g. `2.0.0.dev0` or `2.0.0rc1`."
	)
	# Test with leading backslash
	assert (
	softwrap(
	"""\
	Do you believe in UFOs, astral projections, mental telepathy, ESP, clairvoyance,
	spirit photography, telekinetic movement, full trance mediums, the Loch Ness monster
	and the theory of Atlantis?
	Ah, if there's a steady paycheck in it,
	I'll believe anything you say.
	[From
	Ghostbusters (1984)]
	"""
	)
	== (
	"Do you believe in UFOs, astral projections, mental telepathy, ESP, clairvoyance, "
	"spirit photography, telekinetic movement, full trance mediums, the Loch Ness monster "
	"and the theory of Atlantis?"
	"\n\n"
	"Ah, if there's a steady paycheck in it, I'll believe anything you say."
	"\n\n"
	"[From Ghostbusters (1984)]"
	)
	)
	# Test without leading backslash
	assert (
	softwrap(
	"""
	Do you believe in UFOs, astral projections, mental telepathy, ESP, clairvoyance,
	spirit photography, telekinetic movement, full trance mediums, the Loch Ness monster
	and the theory of Atlantis?
	Ah, if there's a steady paycheck in it,
	I'll believe anything you say.
	[From
	Ghostbusters (1984)]
	"""
	)
	== (
	"Do you believe in UFOs, astral projections, mental telepathy, ESP, clairvoyance, "
	"spirit photography, telekinetic movement, full trance mediums, the Loch Ness monster "
	"and the theory of Atlantis?"
	"\n\n"
	"Ah, if there's a steady paycheck in it, I'll believe anything you say."
	"\n\n"
	"[From Ghostbusters (1984)]"
	)
	)
	assert (
	softwrap(
	"""
	Do you
	believe in:
	UFOs
	astral projections
	mental telepathy
	...
	Ah, if there's a steady paycheck in it,
	I'll believe anything you say.
	"""
	)
	== (
	"Do you believe in:"
	"\n\n"
	" UFOs\n"
	" astral projections\n"
	" mental telepathy\n"
	" ...\n"
	"\n"
	"Ah, if there's a steady paycheck in it, I'll believe anything you say."
	)
	)
	assert (
	softwrap(
	"""
	Do you believe in:
	UFOs
	astral projections
	mental telepathy
	...
	"""
	)
	== (
	"Do you believe in:"
	"\n"
	" UFOs\n"
	" astral projections\n"
	" mental telepathy\n"
	" ..."
	)
	)
	assert (
	softwrap(
	"""
	Roll Call:
	```
	- Dr. Peter Venkman
	- Dr. Egon Spengler
	- Dr. Raymond Stantz
	- Winston Zeddemore
	And not really a ghostbuster, but we need to test wrapped indentation
	- Louis (Vinz, Vinz Clortho,\
	Keymaster of Gozer. Volguus Zildrohar, Lord of\
	the Sebouillia)
	```
	All here.
	"""
	)
	== (
	"Roll Call:\n\n"
	" ```\n"
	" - Dr. Peter Venkman\n"
	" - Dr. Egon Spengler\n"
	" - Dr. Raymond Stantz\n"
	" - Winston Zeddemore\n"
	"\n"
	" And not really a ghostbuster, but we need to test wrapped indentation\n"
	# No \n at the end of this one
	" - Louis (Vinz, Vinz Clortho, Keymaster of Gozer. Volguus Zildrohar, Lord of "
	"the Sebouillia)\n"
	" ```\n"
	"\nAll here."
	)
	)
	assert (
	softwrap(
	f"""
	Roll Call:
	{bullet_list(["Dr. Peter Venkman", "Dr. Egon Spengler", "Dr. Raymond Stantz"])}
	All here.
	"""
	)
	== (
	"Roll Call:\n\n"
	" * Dr. Peter Venkman\n"
	" * Dr. Egon Spengler\n"
	" * Dr. Raymond Stantz\n"
	"\nAll here."
	)
	)
	assert softwrap("A\n\n\nB") == "A\n\nB"
	assert (
	softwrap(
	f"""
	Roll Call:
	{bullet_list(["Dr. Peter Venkman", "Dr. Egon Spengler", "Dr. Raymond Stantz"])}
	All here.
	"""
	)
	== (
	"Roll Call:\n"
	" * Dr. Peter Venkman\n"
	" * Dr. Egon Spengler\n"
	" * Dr. Raymond Stantz\n"
	"All here."
	)
	)
	# This models when we output stdout/stderr. The canonical way to do that is to indent every line
	# so that softwrap preserves common indentation and the output "looks right"
	stdout = "* Dr. Peter Venkman\n* Dr. Egon Spengler\n* Dr. Raymond Stantz"
	assert (
	softwrap(
	f"""
	Roll Call:
	{textwrap.indent(stdout, " "*2)}
	All here.
	"""
	)
	== (
	"Roll Call:\n"
	" * Dr. Peter Venkman\n"
	" * Dr. Egon Spengler\n"
	" * Dr. Raymond Stantz\n"
	"All here."
	)
	)

[kaos]

@benjyw also see the commit message of 571cc15 with the before and after blocks. (the backtick vs indent was just my personal preference for console output.. you may disagree with it :) )

[benjyw]

@benjyw also see the commit message of 571cc15 with the before and after blocks. (the backtick vs indent was just my personal preference for console output.. you may disagree with it :) )

Ah yes, that is why I didn't use softwrap, because I couldn't get the hard linebreaks to work. But that renders fine, I'm not fussy about the backticks, so it's fine.