refactor: add generic wrap Twig filter to dedupe comment-wrapping logic#1586
refactor: add generic wrap Twig filter to dedupe comment-wrapping logic#1586KevalPrajapati wants to merge 3 commits into
wrap Twig filter to dedupe comment-wrapping logic#1586Conversation
…ogic
Introduce a single `wrap(width, prefix)` Twig filter in SDK.php and route the
duplicated comment-wrapping logic through it, removing the near-identical
per-language copies.
Removed filters (all reproduced exactly by `wrap`):
- comment1, comment2 (SDK.php / Web.php)
- comment3 (Web.php) — was dead code, never referenced by any template
- dartComment, dotnetComment, swiftComment, rubyComment
Call sites migrated to `| wrap(75, '<prefix>')` across dart, flutter, dotnet,
unity, swift, apple, ruby, php, deno and react-native templates.
This is a pure no-op cleanup: generated output is byte-identical for all 24
SDKs (verified by diffing before/after generation, not git, since examples/
is gitignored). Adds 5 unit tests for the new filter (none existed before).
Out of scope (deliberate follow-ups):
- Go/Rust filters, which add trim()/indent and would need extra `wrap` options.
- The ~12 templates using inline replace({"\n": ...}); folding them in changes
output (they currently don't word-wrap), so they belong in a separate PR.
Greptile SummaryThis PR consolidates ~8 duplicated comment-wrapping Twig filters into a single generic
Confidence Score: 5/5This is a pure deduplication refactor — the generated SDK output is unchanged and all call sites have been updated correctly. Every removed filter has been replaced by No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "test: drop dedicated wrap filter test" | Re-trigger Greptile |
The helper only emitted filter arguments when a width was passed, so a prefix given without a width was silently dropped. Fall back to the filter's default width (75) whenever a prefix is present.
The filter is a thin wrapper around wordwrap(); a dedicated test isn't warranted. Behaviour is already covered by the unchanged generated output across all SDKs.
Closes #1479 (partially — see scope below).
The same
wordwrap()+ prefix logic was copy-pasted across ~8 comment filters. This adds onewrap(width, prefix)filter inSDK.phpand points the existing call sites at it, then deletes the duplicates.Removed:
comment1,comment2,dartComment,dotnetComment,swiftComment,rubyComment. Also droppedcomment3, which wasn't referenced by any template.Templates now call e.g.
{{ method.description | wrap(75, ' /// ') }}, with the prefix at the call site instead of buried in PHP.No output change
This is a straight refactor — I kept the width at 75 and the exact prefixes, so the generated SDKs come out identical. I verified by generating every SDK before and after the change and diffing the output directories (all 24 match).
git diffalone isn't enough here sinceexamples/is gitignored.Added a few unit tests for the new filter — there weren't any covering this before.
Left for follow-ups
GoandRustalso have their own versions, but theytrim()and take an indent argument, sowrapwould need a couple of extra params. Didn't want to widen the API in the same PR.replace({"\n": ...})templates (Kotlin, Android, Python, etc.) mentioned in the issue — those don't word-wrap today, so switching them towrapwould change their output. That's a behavior change, not a no-op, so it makes more sense as its own PR.Happy to fold those in here instead if you'd prefer one bigger PR.