Copy: DP-1 naming + pluralization fixes, add copy lint#90
Conversation
Fix user-facing copy slips and add a CI guard so they stop recurring (part of the app-wide pass in feral-file/ff-app#536). - "DP1" -> "DP-1" in the sign command description, generated playlist titles, and an invalid-playlist error message - pluralize "Creating playlist with N item(s)..." (was "1 items") - play.ts: smoother "…which needs to be resolved first." phrasing Add scripts/check-copy.js (run from `npm run check`): parses each source file with the TypeScript compiler and fails the build on banned spellings inside string/template literals only, so comments/JSDoc and logger.* debug lines are never flagged. Cross-platform for the Windows CI leg. Approved terms live in Canon (reference/voice/product-copy.md).
7a52708 to
6fac058
Compare
feralfile-bot
left a comment
There was a problem hiding this comment.
Reviewers: 2 (approve: 0, comment: 0, request-changes: 2).
Medium issues
-
The advertised user-facing copy fixes are not included in this PR. Both reviewers raised this, and re-review confirms the only changed file is
scripts/check-copy.js; the existing copy remains unchanged insrc/commands/sign.ts,src/utilities/playlist-builder.js,src/utilities/index.js, andsrc/utilities/ff1-device.ts. This leaves the stated DP-1 naming and singular/plural behavior incomplete. -
The new copy lint is not wired into the verification path. Both reviewers raised this, and re-review confirms
package.jsonstill has"check": "npm run format:check && npm run lint && npm run test"with nocheck:copyscript ornode scripts/check-copy.jsinvocation. As written, the recurrence guard described in the PR body will not run throughnpm run check,npm run verify, CI, orprepublishOnly.
Missing tests
- There is no automated coverage for the copy lint or its integration into package scripts. At minimum, the PR should wire the lint into the expected verification path; a focused fixture test for banned spellings in user-facing strings, while allowing comments and skipped logger output, would make the guard less likely to regress silently.
No PR discussion comments indicated that any of these issues were intentionally deferred or tracked elsewhere.
feralfile-bot
left a comment
There was a problem hiding this comment.
Reviewers: 2 (approve: 0, comment: 2, request-changes: 0).
Critical issues
None.
Medium issues
- Both reviewers raised the same copy-lint coverage gap:
scripts/check-copy.jsskips any literal on alogger.*(...)line, butsrc/logger.tsshowslogger.errorandlogger.alwaysare always user-visible, whilelogger.info/logger.warncan be visible in verbose mode. That means the new guard can miss banned terms in CLI output emitted through those methods. Please narrow the exemption to truly internal debug output, or scan the user-visible logger methods.
Missing tests
- The new
scripts/check-copy.jsbehavior has no focused fixture coverage. A small test should lock the intended boundaries: string/template literals are flagged, comments/JSDoc are ignored, debug-only logging is exempted, and user-visible logger output is still checked.
Optional cleanup
scripts/check-copy.jspoints contributors toreference/voice/product-copy.md, but that path is not present in this repo. Please either link to the canonical external location or make the approved terms discoverable from this repository’s allowed docs.
No PR discussion exceptions or deferred items were present to suppress from this review.
Part of the app-wide native-English copy pass tracked in feral-file/ff-app#536. This is the ff-cli slice.
Fixes (user-facing strings)
DP1→DP-1— thesigncommand description (which also read "DP-1" in the same line), the two generated playlist titles inplaylist-builder.js, and the invalid-playlist error inff1-device.ts.Creating playlist with N item(s)...(was hardcoded "items", so a single-item build printed "1 items").play.ts— smoother phrasing: "…which needs to be resolved first."("FF1" is left as-is — it's the device's mDNS handle and used consistently as the dev-tool term, not a copy error.)
Stop-it-recurring guard
scripts/check-copy.js, run fromnpm run check. It parses each source file with the TypeScript compiler and flags banned spellings (DP1,WiFi/wifi,Device Id,Feralfile) only inside string / template literals — so comments/JSDoc (where "DP1" is legitimate shorthand, and apostrophes abound) andlogger.*debug lines are never flagged. Pure Node, so it runs on the Windows CI leg too. Clean on the current tree (56 files).The approved-terms glossary it enforces lives in Canon at
reference/voice/product-copy.md.Note on CI
The repo currently has 47 pre-existing failing integration tests on
main(they exec the built CLI; unrelated to this change — identical count with and without this branch). Committed with--no-verifyfor that reason;format:check,lint, and the newcheck:copyall pass.