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: update and escape a handful of templates #9258

Conversation

scottbarnes
Copy link
Collaborator

@scottbarnes scottbarnes commented May 12, 2024

We run extract/i18n-messages extract by default now, but we need to get the master branch in a state where the command can run cleanly, or the files in this commit will get flagged with any commit that makes an i18n change.

This seems to be confusing people.

Edit: I double checked by adding another commit with an i18n text change and no accompanying update to messages.pot within the PR. on push, pre-commit ran, failed because message.pot changed by way of the pre-commit hook, added the updated messages.pot to the PR, ran again, and succeeded. This is evidence, though not entirely conclusive, that it may fix things for, e.g., #9246.

Testing

  • Get on the current master branch.
  • Make a change to to some i18n text. E.g.:
diff --git a/openlibrary/templates/type/edition/compact_title.html b/openlibrary/templates/type/edition/compact_title.html
index a9b60b517..189f5fb04 100644
--- a/openlibrary/templates/type/edition/compact_title.html
+++ b/openlibrary/templates/type/edition/compact_title.html
@@ -6,7 +6,7 @@ $def with (book_title, edit_url)
     <a
       class="linkButton linkButton--large"
       href="$edit_url"
-      title="$_('Edit this page')"
+      title="$_('Edit this page please')"
       data-ol-link-track="CTAClick|StickyEdit"
       rel="nofollow"
     >$_("Edit")</a>

Run pre-commit to generate the POT files, and see that it both fails and generates a new i18n/messages.pot:

❯ pre-commit run generate-pot --all-files
Generate POT.............................................................Failed
- hook id: generate-pot
- files were modified by this hook

Failed to extract /home/scott/code/openlibrary/openlibrary/templates/lib/markdown.html: TokenError('unexpected EOF in multi-line statement', (1, 0))
Failed to extract /home/scott/code/openlibrary/openlibrary/templates/lib/not_logged.html: TokenError('unexpected EOF in multi-line statement', (1, 0))
Failed to extract /home/scott/code/openlibrary/openlibrary/macros/DonateModal.html: TokenError('unexpected character after line continuation character', (1, 210))
Failed to extract /home/scott/code/openlibrary/openlibrary/macros/databarAuthor.html: TokenError('unexpected EOF in multi-line statement', (1, 0))
Updated strings written to /home/scott/code/openlibrary/openlibrary/i18n/messages.pot

Stakeholders

@scottbarnes scottbarnes marked this pull request as draft May 12, 2024 17:16
@scottbarnes scottbarnes force-pushed the fix/fix-templates-with-eof-issues branch from 3353757 to 3a39a8b Compare May 12, 2024 17:22
@scottbarnes scottbarnes marked this pull request as ready for review May 12, 2024 17:23
Copy link
Member

@mekarpeles mekarpeles left a comment

Choose a reason for hiding this comment

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

changes lgtm though I think we'll need to rebase or resolve conflicts before proceeding due to other recent i18n changes merged

@mekarpeles mekarpeles added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label May 13, 2024
@mekarpeles mekarpeles self-assigned this May 13, 2024
@scottbarnes scottbarnes force-pushed the fix/fix-templates-with-eof-issues branch from 3a39a8b to ce7c7f7 Compare May 13, 2024 15:58
@github-actions github-actions bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label May 13, 2024
We have some templates that both would error on every i18n change, and
also that weren't included in i18n generation because of those errors.
This commit fixes those templates.
@scottbarnes scottbarnes force-pushed the fix/fix-templates-with-eof-issues branch from ce7c7f7 to e3d69eb Compare May 13, 2024 16:02
@mekarpeles mekarpeles merged commit 1d86a59 into internetarchive:master May 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants