Skip to content

fix up pre-commit for xliff files#19426

Merged
SaschaCowley merged 10 commits intobetafrom
fixXliff
Jan 18, 2026
Merged

fix up pre-commit for xliff files#19426
SaschaCowley merged 10 commits intobetafrom
fixXliff

Conversation

@seanbudd
Copy link
Copy Markdown
Member

@seanbudd seanbudd commented Jan 9, 2026

Link to issue number:

None

Summary of the issue:

Adding new languages to NVDA from Crowdin causes pre-commit to fail, as it commits large xliff files.
Failed job: https://github.com/nvaccess/nvda/actions/runs/20842477976/job/59879416174

Description of user facing changes:

None

Description of developer facing changes:

We can add new languages to NVDA via Crowdin

Description of development approach:

update pre-commit

Testing strategy:

Tested trying to commit a large xliff file locally

Known issues with pull request:

n/a

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@seanbudd seanbudd added this to the 2026.1 milestone Jan 9, 2026
Copilot AI review requested due to automatic review settings January 9, 2026 05:52
@seanbudd seanbudd requested a review from a team as a code owner January 9, 2026 05:52
@seanbudd seanbudd requested a review from SaschaCowley January 9, 2026 05:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a pre-commit configuration issue that prevents adding new languages to NVDA from Crowdin. The check-added-large-files hook was blocking commits of large xliff translation files.

  • Updates pre-commit config to exclude xliff files from the large file size check

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .pre-commit-config.yaml Outdated
Comment thread .pre-commit-config.yaml
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@seanbudd seanbudd marked this pull request as draft January 9, 2026 05:57
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jan 12, 2026
Comment thread .pre-commit-config.yaml Outdated
@seanbudd seanbudd marked this pull request as ready for review January 16, 2026 05:05
@seanbudd seanbudd mentioned this pull request Jan 16, 2026
5 tasks
Copy link
Copy Markdown
Member

@SaschaCowley SaschaCowley left a comment

Choose a reason for hiding this comment

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

Nice one

@SaschaCowley SaschaCowley merged commit fb55abb into beta Jan 18, 2026
39 checks passed
@SaschaCowley SaschaCowley deleted the fixXliff branch January 18, 2026 22:55
seanbudd added a commit that referenced this pull request Jan 19, 2026
was blocked by #19426 and #19458
Summary of the issue:

#19458 introduced Cambodian, but we need to document it's support in NVDA

Additionally, unit tests are temporarily broken by #19458 causing an issue, with python/cpython#123853 as the root cause
Description of user facing changes:

Document cambodian support
Description of developer facing changes:

None
Description of development approach:

    Introduced _LCIDS_TO_TRANSLATED_LOCALES_OVERRIDES in languageHandler.py to correctly map the Windows LCID for Khmer (Cambodian), addressing upstream issues in Python's locale mapping. [1] [2]
    Refactored type annotations throughout languageHandler.py to use modern Python syntax (e.g., str | None instead of Optional[str], list[str] instead of List[str]). [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants