Notifications: remove extra indentation and trailing newlines

[humitos]

I've found myself wanting to write the copy for all the messages in a simple way that avoid indentation and punctuation mistakes and I'm happy with the result so far. However, the downsides they have is they return they messages as \n \n Please try... (note the \n and the extra spaces).

I'd like to remove those automatically somehow without changing the way we write down the copy for these messages if possible.

We discussed this a little at #10922 (comment) where I exposed what I ideally want.

Example in the code:

readthedocs.org/readthedocs/notifications/messages.py

Lines 71 to 78 in 7d1daee

	body=_(
	"""
	There was a problem with Read the Docs while building your documentation.
	Please try again later.
	If this problem persists,
	report this error to us with your build id ({instance.pk}).
	"""
	),

[stsewd]

You can use textwrap.dedent + string.strip.

import textwrap

s = textwrap.dedent(
    """
    Foo

    bar
    """
).strip()

[humitos]

That works great!

However, it seems that textwrap.dedent does not support a translated string? Weird.

build_1        |   File "/usr/src/app/checkouts/readthedocs.org/readthedocs/core/admin.py", line 17, in <module>
build_1        |     from readthedocs.oauth.tasks import sync_remote_repositories
build_1        |   File "/usr/src/app/checkouts/readthedocs.org/readthedocs/oauth/tasks.py", line 15, in <module>
build_1        |     from readthedocs.oauth.notifications import (
build_1        |   File "/usr/src/app/checkouts/readthedocs.org/readthedocs/oauth/notifications.py", line 16, in <module>
build_1        |     Message(
build_1        |   File "/usr/src/app/checkouts/readthedocs.org/readthedocs/notifications/messages.py", line 20, in __init__
build_1        |     self.header = textwrap.dedent(header).strip()
build_1        |   File "/usr/lib/python3.10/textwrap.py", line 438, in dedent
build_1        |     text = _whitespace_only_re.sub('', text)
build_1        | TypeError: expected string or bytes-like object

[agjohnson]

+1 on dedent here

It looks like you are effectively doing textwrap.dedent(_("...")) though

To both make the source easy to work with and also solve the issue of the translations including extraneous whitespace, you probably need _(textwrap.dedent("...")).

[agjohnson]

Noted at #10922 (comment)

However, it might be best to avoid altering our translation files until these strings are resolved. I don't quite know how gettext handles moved strings vs moved and altered strings, but I suspect the additional whitespace could throw off all the existing translations.

I'll add this to this sprint, as I'd like to help translators as much we can here. I don't really know if it matters though. Moving the string is probably already a somewhat breaking change.

[humitos]

It looks like you are effectively doing textwrap.dedent(_("...")) though
To both make the source easy to work with and also solve the issue of the translations including extraneous whitespace, you probably need _(textwrap.dedent("...")).

Yes, I tried doing the first approach because doing _(textwrap.dedent()) means doing it in all the strings instead of just once. However, it seems the shortcut I want to follow is not possible here and I will need to re-write all the strings 😞

[agjohnson]

Yeah, I figured we'd have to do something like this, unfortunately. Multiline strings are always a pain.

I do believe there is magic you can do with gettext and creating your own gettext wrappers, but this is almost certainly way too deep.

[humitos]

I don't know exactly how gettext works internally, but I assumed that if I do _(textwrap.dedent(header).strip()) inside the initializer at

readthedocs.org/readthedocs/notifications/messages.py

Line 25 in 24bd75d

self.header = header

the translation string won't get identified by gettext and exposed to be translated. If that works, we can write this only once, but I'm not sure how to test it.

[humitos]

I understand that we can't translate variables in a simple way:

(The caveat with using variables or computed values, as in the previous two examples, is that Django’s translation-string-detecting utility, django-admin makemessages, won’t be able to find these strings. More on makemessages later.)

(from https://docs.djangoproject.com/en/5.0/topics/i18n/translation/#standard-translation)

I think what I've done here is the way to go, unfortunately 😞