Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update 'dprint fmt' results for .po files #2166

Closed
wants to merge 2 commits into from

Conversation

djmitche
Copy link
Collaborator

I cleared $HOME/.cache/dprint on my machine and re-ran dprint check (which had previously succeeded), and it generated these changes.

Somewhat aside, it's frustrating that dprint is very much a moving target, and moves at a different pace than our PRs. As a concrete example, this PR will also fail due to #2165. And, the check is required to merge, so until that's fixed we can't merge anything.

Copy link
Collaborator

@joaovicmendes joaovicmendes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for pt-BR changes.

@mgeisler
Copy link
Collaborator

mgeisler commented Jul 3, 2024

I cleared $HOME/.cache/dprint on my machine and re-ran dprint check (which had previously succeeded), and it generated these changes.

Oh, that's not good! I haven't noticed problems with the caching myself, but that is of course not a guarantee for anything 😄

Somewhat aside, it's frustrating that dprint is very much a moving target, and moves at a different pace than our PRs. As a concrete example, this PR will also fail due to #2165.

We should fix this so that we get stable and consistent results. The formatting plugins for dprint are already versioned, so I think we simply have to pin the rustfmt binary to a particular nightly build. I can make a PR for that!

@djmitche
Copy link
Collaborator Author

djmitche commented Jul 3, 2024

I suspect this particular issue is actually a msgfmt version issue?

⸩ msgfmt --version
msgfmt (GNU gettext-tools) 0.21

@djmitche
Copy link
Collaborator Author

djmitche commented Jul 3, 2024

Huh, same version in CI

▶ Run msgfmt --version
msgfmt (GNU gettext-tools) 0.21

@djmitche djmitche changed the title Update 'dprint fmt' results Update 'dprint fmt' results for .po files Jul 3, 2024
@djmitche
Copy link
Collaborator Author

djmitche commented Jul 3, 2024

Dprint uses msgcat and not msgfmt but they are both in gettext-utils so the versions are the same.

I can confirm that msgcat - itself will reformat the files as shown in this PR:

⸩ msgcat - < po/de.po > t                                                                                                               
⸩ mv t po/de.po 
⸩ git diff
diff --git po/de.po po/de.po
index 9b2ab992..cfa8f599 100644
--- po/de.po
+++ po/de.po
@@ -4008,4 +4008,4 @@ msgid ""
 msgstr ""
-"Das Hinzufügen von `#`, z. B. `{a:#?}`, ruft ein „hübsches "
-"Druckformat“ (pretty print) auf, das einfacher zu lesen ist."
+"Das Hinzufügen von `#`, z. B. `{a:#?}`, ruft ein „hübsches Druckformat“ "
+"(pretty print) auf, das einfacher zu lesen ist."
 
@@ -8928,4 +8928,4 @@ msgstr ""
 "erstellt hast. Allerdings kannst Du die Werte sowohl von `a` als auch von "
-"`s` sicher lesen. Weitere Einzelheiten werden im Abschnitt "
-"„Ausleihenprüfer“ (borrow checker) erläutert."
+"`s` sicher lesen. Weitere Einzelheiten werden im Abschnitt „Ausleihenprüfer“ "
+"(borrow checker) erläutert."

which suggests that there is some different gettext-utils configuration here?

I wondered if it is simply that the default for [-w](https://www.gnu.org/software/gettext/manual/gettext.html#index-_002dw_002c-msgcat-option) is based on the screen width somehow, and that differs between CI and my terminal. However, trying a number of -w values locally does not reproduce the wrapping currently in main. The closest is -w 79, but that still leaves the diff shown above.

Perhaps this has something to do with the calculation of unicode line widths? Setting LC_ALL to various things (C, en_US.UTF-8, es_ES.UTF-8, etc.) doesn't seem to affect the output.

Other ideas?

@djmitche
Copy link
Collaborator Author

djmitche commented Oct 3, 2024

See #2173.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants