Repository navigation
Keep plugin manifests ASCII-only; add check_manifests_ascii guard - #578
Merged
Merged
Conversation
Eight manifests on main carried raw UTF-8 (em dashes, and baseball's up/down arrows): clock-simple, ledmatrix-weather, pomodoro-timer and five scoreboards (baseball, basketball, football, lacrosse, soccer; added in #570/#571). Tools that open manifest.json in the platform default encoding -- cp1252 on Windows: cores before 3.4.0 and the core's scripts/dev_server.py -- garble those bytes, or raise UnicodeDecodeError on characters whose UTF-8 includes a byte cp1252 leaves undefined. Every other manifest already stores such characters as JSON escapes. - Rewrite the eight with a text-level substitution to JSON escape sequences (formatting, key order and line endings unchanged; json.loads before == after), and PATCH-bump each with a new top versions[] entry. plugins.json synced by update_registry.py. - scripts/check_manifests_ascii.py: fails on any raw non-ASCII byte in plugins/*/manifest.json, naming line, column and the escape to write; also flags a BOM or a legacy-encoded byte, and fails if it finds 40 or fewer manifests. - scripts/test_check_manifests_ascii.py: regression suite, which also runs the guard over the real tree so CI's scripts/test_*.py glob enforces it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 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. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 25 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eight manifests on
maincontained raw UTF-8: em dashes, plus baseball's up/down arrows. Every other manifest stores such characters as JSON\uXXXXescapes. This PR converts the eight to escapes and adds a guard so it doesn't happen again. Nothing on the display changes.Why it matters
Anything that opens
manifest.jsonwithoutencoding=uses the platform's default encoding, which is cp1252 on Windows. That includes LEDMatrix cores before v3.4.0 (load_manifest, used bycheck_plugin.py) and the core'sscripts/dev_server.pyonmaintoday. Under cp1252:—.UnicodeDecodeError. A closing curly quote, U+201D, is one.This has already happened: on-air's description and odds-ticker's 1.1.7 notes carry escaped mojibake (
\u00e2\u20ac\u201d) from an earlier round trip. That repair is separate work.Changes
The eight manifests: clock-simple, ledmatrix-weather, pomodoro-timer, and the baseball, basketball, football, lacrosse and soccer scoreboards. The scoreboard ones came in with scoreboards: require core 3.6.1; drop the bundled favourite-check and timezone code (stage 2 sunset) #570/soccer: use core's favourite-team check (2.37.0, requires core 3.6.2) #571. Each non-ASCII character becomes its JSON escape, with surrogate pairs above the BMP.
json.loads(before) == json.loads(after)was asserted for every file.Versions: each plugin gets a PATCH bump with a new top
versions[]entry.ledmatrix_min_versionis copied from the previous top entry, and the notes explain the change.plugins.jsonwas synced withupdate_registry.py.scripts/check_manifests_ascii.py(new guard): reads eachplugins/*/manifest.jsonas raw bytes. It prints aFAILline for every non-ASCII character, giving the line, column and escape to write. It also reports a BOM or a byte that isn't valid UTF-8. It fails if it finds fewer than 41 manifests, so a moved tree can't pass by finding nothing. Exit codes: 0 pass, 1 fail.scripts/test_check_manifests_ascii.py(regression suite, 27 checks): runs the guard over the real tree, which is how CI'sscripts/test_*.pyglob enforces it. It also covers:str.splitlines()would treat as line breaksmanifest.jsonbeing ignoredTest plan
python scripts/check_manifests_ascii.py: PASS on this branch (46 manifests). Run against unfixedmain, it lists all eight files by line and column.python scripts/test_check_manifests_ascii.py: all 27 checks pass.python update_registry.py --check: PASS.python scripts/check_version_bump.py --base origin/main <the 8 ids>: all PASS.python scripts/check_manifest_version_fields.py <the 8 ids>: OK.main'sschema/manifest_schema.json.test_check_version_bump.py,test_update_registry.py,test_check_manifest_version_fields.py: pass.Notes for reviewer
mainread"2.7.8\r". Regenerating it clears the stray CR, which also appears in ten otherlatest_versionvalues (from Remove code nothing calls (20 plugins) #560). A separate change fixes the rest and teachesupdate_registry.py --checkto catch it, so that PR may conflict on the weather line.version.🤖 Generated with Claude Code