Skip to content

fix(gottext): name a translation whose tags differ from its message - #135

Merged
SirLouen merged 3 commits into
gopherium:mainfrom
CloudCorpRecords:fix/133-gottext-tags
Nov 8, 2026
Merged

SirLouen merged 3 commits into
gopherium:mainfrom
CloudCorpRecords:fix/133-gottext-tags

Conversation

@CloudCorpRecords

@CloudCorpRecords CloudCorpRecords commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #133

mismatched in gottext compared a translation's placeholders with the ones its message names, but never looked at the tags, so a translation that added, dropped, renamed or repeated a tag passed every catalogue check and then rendered wrong under createInterpolateElement.

This change:

  • Adds a tag reader in gottext/src/gates.ts using the same pattern createInterpolateElement uses, /<(\/)?(\w+)\s*(\/)?>/g, keeping the closing slash and the self-closing slash as part of each tag, so an opening, a closing and a self closing tag count as different tags and spaces inside a tag do not count.
  • Compares the tags of each translated form with the tags of its message (the singular message for form 0, the plural message for the rest) as a multiset, the way placeholders are compared today, so order stays free but a repeated tag is named.
  • Leaves an empty form passing, as with placeholders, and keeps the issue's out-of-scope case passing: a translation that closes a tag before opening it holds the same tags.
  • Adds the cases to gottext/test/build.test.ts and a changelog entry under Unreleased in gottext/CHANGELOG.md.

Scope note: one "Done when" item in #133 asks for a line in translations/health.md in the separate docs repo. That item is not addressed here, because it does not live in this repo; everything in the issue that lives in this repo (the check, the tests, the changelog entry) is included. The catalogues that use gottext today already pass this check, so no translation changes.

Verification, all from gottext on this branch:

  • Before the fix, the new tag tests failed at main: npx vitest run test/build.test.ts — 95 tests, 8 failed (every "names a translation … tag" case returned [] instead of the message key), 87 passed. The pristine baseline of the same file at main was 83 passed.
  • After the fix: npx vitest run — 10 test files, 255 passed, including the 95 in build.test.ts (the 12 new tag cases among them).
  • pnpm run typecheck — passes.
  • pnpm run lint — passes (oxlint and eslint, 0 warnings, 0 errors).

AI assistance: the change was drafted with an AI coding agent; every command above was run against this branch.

Summary by CodeRabbit

  • Bug Fixes
    • Translation validation now detects markup tag mismatches, including added, missing, renamed, repeated, or differently formatted tags. Checks also cover the corresponding singular or plural source form. Tags may move within a translation, whitespace before a tag’s closing angle bracket is allowed, and empty translations remain valid.

@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

mismatched now checks tags as well as placeholders in translated forms. It checks singular and plural forms, accepts reordered tags and empty forms, and ignores whitespace inside tags.

Changes: Tag mismatch validation

Layer / File(s) Summary
Compare tags in translated forms
gottext/src/gates.ts, gottext/test/build.test.ts, gottext/CHANGELOG.md
mismatched compares sorted tag lists for source messages and translated forms. Tests cover added, dropped, renamed, repeated, and self-closing tag differences, plus accepted cases. The changelog documents the check.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: sirlouen

Merge Risk: Minimal · up to ac426

Tag mismatches are now checked alongside placeholders. No merge-blocking issue was identified.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the main change: the gottext catalogue check now flags a translation whose tags differ from its message. The raw summary says the mismatched check compares tags and the changelog entry…
Linked Issues check Passed Issue #133 requires that mismatched detect added, dropped, renamed, or repeated tags in the singular and every plural form. It also requires that moved tags, tags with inner spaces, and empty forms …
Out of Scope Changes check Passed The whole-PR diff from merge base 120287f to head ac426e2 changes three files. gottext/src/gates.ts adds the tag reader and answersTags, and it calls them from mismatched. The docblock edits…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check

Explanation

The title names the main change: the gottext catalogue check now flags a translation whose tags differ from its message. The raw summary says the mismatched check compares tags and the changelog entry describes the same behavior, so the title matches the changeset. The title is specific and uses the conventional fix(gottext) prefix. It is 67 characters, which is within a normal length.

Full details: Linked Issues check

Explanation

Issue #133 requires that mismatched detect added, dropped, renamed, or repeated tags in the singular and every plural form. It also requires that moved tags, tags with inner spaces, and empty forms pass, and that close-before-open passes. Tests are required in gottext/test/build.test.ts, plus a changelog entry.

Source evidence at head (gottext/src/gates.ts): TAG = /&lt;(\/)?(\w+)\s*(\/)?&gt;/g matches the createInterpolateElement pattern named in the issue. tags() keeps the closing slash and the self-closing slash in each tag key, so &lt;br/&gt;, &lt;br&gt;, and &lt;/br&gt; are distinct. It sorts the keys, so comparison is a multiset check, and order is free. answersTags() returns true for an empty form. mismatched() checks each form against msgid for form 0 and msgid_plural for the rest, and requires both the placeholder check and the tag check to pass.

Tests at head (gottext/test/build.test.ts) cover: added, dropped, renamed, and repeated tags; self-closing versus opening tags in both directions; a tag name with capitals and digits; a plural form that drops a tag; a singular form that repeats a tag; per-form plural comparison; a moved tag; spaces inside tags (&lt;notes &gt;, &lt;/notes &gt;); an empty form; and a close-before-open translation that passes. Each required case has a test.

gottext/CHANGELOG.md has an Unreleased Fixed entry. The translations/health.md item lives in a separate docs repository and is non-coding, so it does not apply to this assessment.

I checked the regex and comparison logic by reading the source. I did not run the test suite. The verdict rests on the source and test code at the reviewed head.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff from merge base 120287f to head ac426e2 changes three files. gottext/src/gates.ts adds the tag reader and answersTags, and it calls them from mismatched. The docblock edits in that file describe the same change. gottext/test/build.test.ts adds tests for the tag behavior only. gottext/CHANGELOG.md adds a Fixed entry under Unreleased that describes the change. Each file connects directly to issue #133. The placeholder logic (answersPlaceholders, placeholders, bare) is unchanged apart from a docblock wording change in alike. No unrelated production behavior or unrelated file appears in the pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: ceefad0f-c67f-4276-82ec-aadd0c01caca
Commits

Reviewing files that changed from the base of the PR and between ccb8b90 and ac426e2.

Files selected for processing: 3

  • gottext/CHANGELOG.md
  • gottext/src/gates.ts
  • gottext/test/build.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

codescene-access[bot]

This comment was marked as outdated.

@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defects were found.

Summary

mismatched now compares tags as well as placeholders.

  • Catalogue checks now flag translations with different tags.

No actionable issues were found.

Reviews (2) · Last reviewed commit: "docs(gottext): trim the tag docblocks an..." · Reviewed by Greptile

@SirLouen
SirLouen force-pushed the fix/133-gottext-tags branch from e8a4beb to ccb8b90 Compare November 7, 2026 23:44
codescene-access[bot]

This comment was marked as outdated.

@SirLouen

SirLouen commented Nov 8, 2026

Copy link
Copy Markdown
Member

Thanks for this.

I rebased your branch on main and shortened your commit message to its first line, so please reset to it before you push again. I also pushed two small commits on top so we can merge it, you don't need to do anything further.

  • Added tests for a closing tag typed as an opening one, a closing tag where the message has a self closing one, tag names with capitals and digits, and each plural form read against its own message. The moved-tag test now swaps two tags, and the spaces test uses two spaces.
  • Cut the tags docblock to one line, made the alike docblock cover tag lists too, and shortened the changelog entry to one line.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@SirLouen
SirLouen merged commit 51444a5 into gopherium:main Nov 8, 2026
45 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.

Stop gottext passing a translation whose tags differ from its message

2 participants