Skip to content

Add options to format-po formatter - #5

Merged
k0d13 merged 2 commits into
mainfrom
kodie/format-po-reference-options
Apr 22, 2026
Merged

Add options to format-po formatter#5
k0d13 merged 2 commits into
mainfrom
kodie/format-po-reference-options

Conversation

@k0d13

@k0d13 k0d13 commented Mar 30, 2026

Copy link
Copy Markdown
Owner
  • Add options to format-po formatter
  • Add ability to not add references
  • Make line numbers opt-in
  • Refactor carbon saykit config to showcase different models

Summary by CodeRabbit

Release Notes

  • New Features

    • Added configurable options for PO file reference formatting, allowing control over whether line number references are included in translation entries.
  • Chores

    • Updated example project structure to reflect simplified locale file organisation.

@changeset-bot

changeset-bot Bot commented Mar 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8642f75

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@saykit/format-po Patch
@saykit/config Patch
babel-plugin-saykit Patch
unplugin-saykit Patch
@saykit/transform-js Patch
@saykit/transform-jsx Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added examples Updates or additions to example apps package: format-po Related to @saykit/format-po labels Mar 30, 2026
@coderabbitai

coderabbitai Bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

The pull request introduces formatter options for the @saykit/format-po package to make PO line number references opt-in. It also reorganises the locale file structure in the carbon-tsdown example from directory-based to filename-based organisation, removing line number information from PO reference comments throughout.

Changes

Cohort / File(s) Summary
Changeset Entry
.changeset/bright-foxes-invite.md
Added changeset documenting patch release for @saykit/format-po with formatter options for PO references and opt-in line numbers.
Example Project Configuration
examples/carbon-tsdown/saykit.config.ts, examples/carbon-tsdown/src/i18n.ts, examples/carbon-tsdown/.gitignore
Updated locale file output path from src/locales/{locale}/messages.{extension} to src/locales/{locale}.{extension}, adjusted import paths and .gitignore pattern to match single-directory structure.
PO Locale Files
examples/carbon-tsdown/src/locales/en.po, examples/carbon-tsdown/src/locales/fr.po
Updated source-reference metadata to remove line number suffixes, converting specific line references (e.g., file.ts:17) to file-only references (e.g., file.ts).
Formatter Options Implementation
packages/format-po/src/formatter.ts
Added FormatterOptions interface and updated createFormatter() to accept optional configuration. Implemented conditional reference handling: references can be disabled, stripped of line numbers by default, or retained with lineNumbers: true option.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

examples

Poem

🐰 With whiskers twitching and a hop so bright,
Our formatter hops to new heights in sight!
Locale files dance in paths so neat,
References optional—control supreme!
Line numbers begone, simplicity's sweet. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: introducing configurable options to the format-po formatter, which is the core purpose reflected across all modified files.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kodie/format-po-reference-options

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@k0d13
k0d13 marked this pull request as draft March 30, 2026 08:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
examples/carbon-tsdown/saykit.config.ts (1)

10-11: Consider demonstrating the new formatter options.

The output path change is fine, but since this PR introduces formatter options, this example could showcase them explicitly:

formatter: createFormatter({ references: { lineNumbers: true } }),

or

formatter: createFormatter({ references: false }),

This would help users discover the new functionality.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/carbon-tsdown/saykit.config.ts` around lines 10 - 11, Update the
example config to show the new formatter options by passing an options object to
createFormatter in the formatter property; specifically demonstrate at least one
of the new options such as createFormatter({ references: { lineNumbers: true }
}) or createFormatter({ references: false }) so users can see how to enable line
numbers for references or disable references entirely when using the formatter
symbol createFormatter in the example config.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/format-po/src/formatter.ts`:
- Around line 69-74: The code uses message.references.map(...) which can throw
if message.references is undefined; change the mapping to use the local variable
references (assigned on line 70) so the code becomes references = (typeof
options.references !== 'object' || options.references.lineNumbers !== true) ?
references.map(r => r.split(':')[0]!) : references; then deduplicate and sort
into item.references as before (ensure you reference the variables
options.references, message.references, references, and item.references when
making the change).

---

Nitpick comments:
In `@examples/carbon-tsdown/saykit.config.ts`:
- Around line 10-11: Update the example config to show the new formatter options
by passing an options object to createFormatter in the formatter property;
specifically demonstrate at least one of the new options such as
createFormatter({ references: { lineNumbers: true } }) or createFormatter({
references: false }) so users can see how to enable line numbers for references
or disable references entirely when using the formatter symbol createFormatter
in the example config.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a1e2c2b-272a-4df2-b778-976672a05904

📥 Commits

Reviewing files that changed from the base of the PR and between 7b75d7c and b63daa5.

📒 Files selected for processing (7)
  • .changeset/bright-foxes-invite.md
  • examples/carbon-tsdown/.gitignore
  • examples/carbon-tsdown/saykit.config.ts
  • examples/carbon-tsdown/src/i18n.ts
  • examples/carbon-tsdown/src/locales/en.po
  • examples/carbon-tsdown/src/locales/fr.po
  • packages/format-po/src/formatter.ts

Comment thread packages/format-po/src/formatter.ts
@k0d13
k0d13 force-pushed the kodie/format-po-reference-options branch from b63daa5 to 1b55b5b Compare April 22, 2026 17:56
@vercel

vercel Bot commented Apr 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
saykit Skipped Skipped Apr 22, 2026 6:06pm

@k0d13

k0d13 commented Apr 22, 2026

Copy link
Copy Markdown
Owner Author

@greptile review

@k0d13
k0d13 marked this pull request as ready for review April 22, 2026 17:57
@greptile-apps

greptile-apps Bot commented Apr 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds configurable options to the createPoFormatter function in packages/format-po/src/formatter.ts, allowing consumers to suppress references entirely (references: false) or strip line numbers from references (references: { lineNumbers: false }). The implementation is clean and well-typed.

Notable changes beyond the stated options:

  • References are now deduplicated (new Set) and sorted before being written — this is an implicit behaviour change for all existing callers, even without passing any options. It is generally an improvement (deterministic output, no duplicates), but it may produce different output compared to the previous release.

One concern flagged:

  • The line-number stripping logic uses r.split(':')[0], which is fragile on paths containing colons (e.g. Windows absolute paths). A regex like r.replace(/:\\d+$/, '') would be safer.

Confidence Score: 5/5

Safe to merge — the feature works correctly for the common Unix-path use case; the one concern is a minor robustness improvement.

The logic is straightforward and correct for the targeted environments. The colon-split issue only affects Windows absolute paths, which are very rarely seen in PO file references. The implicit deduplication/sorting change is a net positive. Overall this is a low-risk, well-structured patch.

No files require special attention; the single concern is isolated to line 72 of packages/format-po/src/formatter.ts.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/format-po/src/formatter.ts
Line: 72

Comment:
**Fragile reference path stripping**

`r.split(':')[0]` splits on the first colon, which works for typical Unix-style `path/to/file.ts:42` references. However, it would silently truncate paths that contain a colon (e.g. Windows absolute paths like `C:\src\file.ts:42``"C"`). A more robust approach would be to strip only a trailing `:\d+` pattern:

```suggestion
          references = references.map((r) => r.replace(/:\d+$/, ''));
```

This ensures only the line number suffix is removed, leaving all other path characters intact.

```suggestion
          references = references.map((r) => r.replace(/:\d+$/, ''));
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Add format-po reference options" | Re-trigger Greptile

Comment thread packages/format-po/src/formatter.ts Outdated
@k0d13
k0d13 merged commit 923fc09 into main Apr 22, 2026
9 checks passed
@k0d13
k0d13 deleted the kodie/format-po-reference-options branch April 22, 2026 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples Updates or additions to example apps package: format-po Related to @saykit/format-po

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant