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 Sphinx Lint for Spanish translations #382

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Sep 18, 2024

Pull Request

Pull Request Checklist

  • Read and followed this repo's Contributing Guidelines
  • Based your PR on the latest version of the correct branch (master or 4.x)
  • Checked your writing carefully for correct English spelling, grammar, etc
  • Described your changes and the motivation for them below
  • Noted what issue(s) this pull request resolves, creating one if needed

Description of Changes

Sphinx Lint's own CI runs on some "friend projects", including this one, and has started failing:

E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/troubleshooting.po:301: found an unbalanced inline literal markup. (unbalanced-inline-literals-delimiters)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/troubleshooting.po:301: found an unbalanced inline literal markup. (unbalanced-inline-literals-delimiters)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/troubleshooting.po:601: default role used (hint: for inline literals, use double backticks) (default-role)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/troubleshooting.po:301: missing space before default role: 'ete``spyder-kernels``'. (missing-space-before-default-role)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/troubleshooting.po:601: superfluous backtick in front of role (backtick-before-role)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/panes.po:33: default role used (hint: for inline literals, use double backticks) (default-role)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/panes.po:247: role with no backticks: " :guilabel:'Tema " (role-without-backticks)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/panes.po:541: role with no backticks: " :guilabel:'Asociaciones " (role-without-backticks)
E         + sphinx-lint/tests/fixtures/friends/spyder-docs/doc/locales/es/LC_MESSAGES/panes.po:1094: role with no backticks: ' :guilabel:. ' (role-without-backticks)

https://github.com/hugovk/sphinx-lint/actions/runs/10919403051/job/30306982773

I've not checked why the spyder-docs CI didn't find these, maybe they don't run on .po files?

Most of these changes are straightforward mechanical fixes, but I don't speak Spanish so please check this one where I deleted a "de":

-msgstr "Finalmente, puedes guardar los datos para una ejecución dada en el disco como un archivo con el ``.Result`` usando el botón :guilabel:`Guardar información de perfilado`. Esto se puede cargar para comparar con una ejecución anterior del mismo archivo usando el botón 'Cargar datos' de :guilabel:. Para eliminar los datos cargados, haga clic en el botón :guilabel:'Limpiar comparación'."
+msgstr "Finalmente, puedes guardar los datos para una ejecución dada en el disco como un archivo con el ``.Result`` usando el botón :guilabel:`Guardar información de perfilado`. Esto se puede cargar para comparar con una ejecución anterior del mismo archivo usando el botón :guilabel:`Cargar datos`. Para eliminar los datos cargados, haga clic en el botón :guilabel:`Limpiar comparación`."

Issue(s) Resolved

n/a

@CAM-Gerlach
Copy link
Member

Hey @hugovk , thanks! I didn't know Sphinx-Lint could check reST embedded in .po files—TIL, that's good to know.

As for these fixes, we ultimately need to make them in our community translation platform, Crowdin, which then gets synced back here. However, it should be okay to just commit this, so long as we also update Crowdin either manually or by actually uploading the modified files (normally the sync only goes one way, repo rst -> pot -> en po -> Crowdin -> es/etc. po -> build -> site, so we don't yet have automation for syncing manual changes here back to Crowdin—but that might be a good idea, since it is quite a pain to do manually string by string as we've had to in the past.

The reason our checks don't do so by default is Sphinx-Lint's pre-commit hook config only has it check reST files—for now we can just add that to the local pre-commit config, though you could add po files there upstream if you want that to be the default behavior.

Coincidentally, I just pushed Sphinx-Lint v1.0.0 to Conda-Forge and was just now working on updating our Pre-Commit hooks for this repo, including Sphinx-Lint, so I can add a check for .po files to the list checked by this hook as part of that PR.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @hugovk !

@CAM-Gerlach CAM-Gerlach merged commit 6e157d9 into spyder-ide:master Sep 18, 2024
4 checks passed
@hugovk hugovk deleted the fix-sphinx-lint branch September 19, 2024 05:47
@CAM-Gerlach
Copy link
Member

Translation fixes synced to Crowdin

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