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

fix(translations): use trimmed in all blocktrans blocks TASK-1553 #5527

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

rgraber
Copy link
Contributor

@rgraber rgraber commented Feb 17, 2025

🗒️ Checklist

  1. run linter locally
  2. update all related docs (API, README, inline, etc.), if any
  3. draft PR with a title <type>(<scope>)<!>: <title> TASK-1234
  4. tag PR: at least frontend or backend unless it's global
  5. fill in the template below and delete template comments
  6. review thyself: read the diff and repro the preview as written
  7. open PR & confirm that CI passes
  8. request reviewers, if needed
  9. delete this section before merging

📖 Description

Developer-only changes

💭 Notes

This was done with a simple find-and-replace. To test it, I compiled the messages and ran
find . -name django.po | xargs awk '{ if (lastline == "msgid \"\"" && $0 ~ "\"\\\\n\"") {print FILENAME, $0} {lastline=$0}}' | wc -l and checked to make sure the output was empty.

The awk command looks for sections of a .po file that look like

msgid ""
"\n"

, which is what it looks like when a translation string begins with a newline char. The full command lists all po files, checks for that sequence of lines using awk for each one, and counts the total. On main, the count was 257. Using this branch, the count is 2. It should be 0, but for some reason makemessages had trouble with zh-Hant and zh-Hans so it didn't actually update those files. That doesn't matter for this PR.

👀 Preview steps

  1. In the kpi container run python ./manage makemessages -a
  2. Locally, cd to the locale directory inside kpi
  3. run find . -name django.po | xargs awk '{ if (lastline == "msgid \"\"" && $0 ~ "\"\\\\n\"") {print FILENAME, $0} {lastline=$0}}' | wc -l
  4. 🔴 [on main] The result will be > 200
  5. 🟢 [on PR] The result will be 0 (or 2, if django has trouble compiling zh-Hant and zh-Hans. Not sure if that was a local issue).

@rgraber rgraber changed the title fix: use trimmed in all blocktrans blocks fix(translations): use trimmed in all blocktrans blocks TASK-1553 Feb 17, 2025
@rgraber rgraber marked this pull request as ready for review February 17, 2025 15:12
@rgraber rgraber self-assigned this Feb 17, 2025
@rgraber rgraber force-pushed the rsgraber/TASK-1553-use-trimmed-in-block-translate branch from 955fbc3 to e4d7662 Compare February 18, 2025 16:20
@noliveleger
Copy link
Contributor

@rgraber I guess in your preview steps you meant:

python ./manage.py makemessages -a

@noliveleger noliveleger requested review from noliveleger and removed request for olive-KTB, jamesrkiger, Akuukis and pauloamorimbr February 18, 2025 16:41
Copy link
Contributor

@noliveleger noliveleger left a comment

Choose a reason for hiding this comment

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

Not tested through Transifex but it did the job in POedit.

@rgraber rgraber merged commit 826a3c8 into main Feb 18, 2025
4 checks passed
@rgraber rgraber deleted the rsgraber/TASK-1553-use-trimmed-in-block-translate branch February 18, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants