Skip to content

Fix false positive i18n warnings for translated glossary term references - #14357

Open
m-aciek wants to merge 16 commits into
sphinx-doc:masterfrom
m-aciek:copilot/fix-last-four-cases-issue-14162
Open

Fix false positive i18n warnings for translated glossary term references#14357
m-aciek wants to merge 16 commits into
sphinx-doc:masterfrom
m-aciek:copilot/fix-last-four-cases-issue-14162

Conversation

@m-aciek

@m-aciek m-aciek commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Purpose

Fix false positive translation warnings introduced by the reference comparison in #14001, following the adjustments in #14144 and #14172.

Translators should be able to translate hyperlink display text and glossary term names without receiving an “inconsistent references” warning when the references still identify the same targets.

For example, translating :term:`text encoding` to :term:`kodowanie tekstu` should be accepted when both names identify the same glossary entry. Changing the reference to a different glossary entry should still warn.

This addresses the remaining cases in #14162.

Changes

  • Compare named hyperlinks by their target IDs after applying the existing reference-name repair. Translated display text and unambiguous translated names are accepted; ambiguous assignments still warn.
  • Compare glossary references by the glossary entries they identify, allowing translated term names and case-insensitive lookup.
  • Defer comparisons involving glossary terms until all documents have been read and their translated terms registered. These checks also work for included documents in singlehtml and LaTeX builds.
  • Preserve checks for changed targets, roles, domains, and reference multiplicity. Reference ordering remains unrestricted, and # noqa remains supported.

Validation

Regression coverage includes translated display text and names, embedded newlines in term targets, reordered references, incorrect target changes, missing or duplicated references, ambiguous named links, and # noqa.

Tests also cover glossaries in other documents, parallel reading, singlehtml and LaTeX document assembly, and rebuilds using saved doctrees.

Local validation: 157 tests passed across the i18n, transforms, and standard-domain suites with Python warnings treated as errors. Ruff, mypy, Pyright, and ty passed for the changed Python files.

Developed with assistance from Copilot and Codex; reviewed and tested.

References

Closes #14162.

Copilot AI and others added 5 commits March 26, 2026 19:32
- Modified update_refnamed_references() to only check reference count, not display text
- This allows translators to change display text while keeping the same target
- Added comprehensive test case covering cases 2-4 from issue sphinx-doc#14162
- Case 5 (glossary terms) already works correctly with explicit target syntax

Co-authored-by: m-aciek <9288014+m-aciek@users.noreply.github.com>
- Clarified comment about display text affecting rawsource comparison
- Simplified case 4 test to better demonstrate the fixup mechanism

Co-authored-by: m-aciek <9288014+m-aciek@users.noreply.github.com>
…ary terms

- Modified update_pending_xrefs() to only check count, not reftarget content
- This allows translators to translate glossary term names when glossary is also translated
- Handles newlines from PO file wrapping without false warnings
- Added comprehensive test cases covering all scenarios from the issue
- All 43 intl tests passing

Agent-Logs-Url: https://github.com/m-aciek/sphinx/sessions/5401f5a5-02a7-43cc-9525-a8039f9fa3f7

Co-authored-by: m-aciek <9288014+m-aciek@users.noreply.github.com>
@m-aciek
m-aciek marked this pull request as draft March 27, 2026 10:23
@m-aciek

m-aciek commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

ty failures should be addressed by #14296.

@m-aciek
m-aciek marked this pull request as ready for review March 27, 2026 10:55
@m-aciek

m-aciek commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

I wonder if revert of #14001 wouldn't be a better alternative. I need yet to sit down to it.

@m-aciek
m-aciek marked this pull request as draft June 27, 2026 21:44
@m-aciek m-aciek mentioned this pull request Jul 26, 2026
@m-aciek
m-aciek marked this pull request as ready for review September 10, 2026 09:55
@m-aciek

m-aciek commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

After looking into this further, I think keeping the checks and fixing the false positives is preferable. Reverting #14001 would also remove useful warnings about incorrect references, including target substitutions that preserve the number of references.

The updated implementation accepts translated display text and glossary names when they identify the same targets, while retaining warnings for actual inconsistencies. It also preserves the citation and footnote checks. This addresses the false positives without losing those diagnostics.

The PR description is updated and reflects current state.

@m-aciek

m-aciek commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

No failures attributable to this PR's changes on head 2af4896f6, compared with base e44a40eb2.

CI failures Cause and verification
15 jobs: the same six tests fail Pygments 2.21.0 changes HTML quote escaping. All six fail on both the base and PR head; all pass with Pygments 2.19.2.
LaTeX: 51 failures Existing incompatibilities with Docutils's development version, plus those Pygments failures. Reproduced on the base.
Docutils HEAD: 53 failures Reproduced 52 on both revisions using CI's exact Docutils commit, 335c924f838ac9180231cf2164f301cc6f6b4ca1. The remaining Python-domain assertion passes locally on both, so its cause remains unconfirmed.

The Docutils failures include string/integer mismatches in writers, changed doctest behavior, and missing output files after earlier build failures. See the CI run.

All 41 tests added by this PR pass in the standard and Docutils HEAD jobs. Ruff, mypy, Pyright, ty, and documentation checks also pass. See the lint run.

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.

i18n: False positive build warnings of hyperlinks and term roles

2 participants