Ensure Ruff Format handles gettext comments correctly

seanbudd commented 3 months ago

issue

Python code using gettext comments can associate in-line comments with gettext. Like this:

# Translators: foo is programming term to represent a sample string
_("Foo")

_(
  # Translators: foo is programming term to represent a sample string
  "Foo"
)

pgettext(
  "sampleStrings",
  # Translators: foo is programming term to represent a sample string
  "Foo"
)

ruff format will reformat something like this

# Translators: foo is programming term to represent a sample string
(_("Foo"), "reallylongstringlongerthanlinelength")

# Translators: foo is programming term to represent a sample string
_("foo, reallylongstringlongerthanlinelength")

# Translators: foo is programming term to represent a sample string
pgettext("tag", "foo, reallylongstringlongerthanlinelength")

to

# Translators: foo is programming term to represent a sample string
(
   _("Foo"),
  "reallylongstringlongerthanlinelength"
)

# Translators: foo is programming term to represent a sample string
_(
   "foo, reallylongstringlongerthanlinelength"
)

# Translators: foo is programming term to represent a sample string
pgettext(
  "tag",
  "foo, reallylongstringlongerthanlinelength"
)

instead of the expected

(
   # Translators: foo is programming term to represent a sample string
   _("Foo"),
  "reallylongstringlongerthanlinelength"
)

_(
   # Translators: foo is programming term to represent a sample string
   "foo, reallylongstringlongerthanlinelength"
)

pgettext(
  "tag",
  # Translators: foo is programming term to represent a sample string
  "foo, reallylongstringlongerthanlinelength"
)

Suggested fix

expand the flake8-gettext config to set a translator comment flag e.g. "Translators:" If this is set, move any comments while formatting, to keep the comments on the line above the target string.

keywords

gettext, translators, translations, translate, i18n, l10n, format, comments

last tested ruff version

v0.5.0

MichaReiser commented 3 months ago

Hmm interesting. I can see how this is frustrating. Do I understand it correctly that this is an opt-in feature for gettext?

I'm hesitant to add support for this because:

It would need to be an opt-in feature
The formatter currently doesn't know if e.g. _ is a gettext call. We could use some very cheap heuristics based on the function names but that won't be bullet prove (another reason to make this opt-in)
Comments are hard and it being an opt-in feature means we'll get almost zero test exposure.

seanbudd commented 3 months ago

Yes this is an opt-in part of gettext, it is fairly commonly used though.

I agree it should be an opt-in feature, but I think you'll get more than zero exposure as this style for gettext usage isn't exactly uncommon. I do agree though outside of basic unit tests it will be hard to truly be certain about every edge case. I think there is already some level of tracking of function names in flake8-gettext rules e.g. https://github.com/astral-sh/ruff/blob/3ce8b9fcae66435285e6cd3d9259912a1f5c4768/crates/ruff_linter/src/rules/flake8_gettext/settings.rs#L14

astral-sh / ruff