mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-04-11 13:23:00 +00:00
docs: address CodeRabbit review comments on #306
Reviewed all 12 CodeRabbit comments on PR #306, verified each against the current code, and fixed the 11 valid ones. The 12th finding is a real code bug (cache_manager.delete() calls in api_helper.py and resource_monitor.py) that's already in the planned follow-up code-fix PR, so it stays out of this docs PR. Fixed: .cursor/plugins_guide.md, .cursor/README.md, .cursorrules - I claimed "there is no --emulator flag" in 3 places. Verified in run.py:19-20 that the -e/--emulator flag is defined and functional (it sets os.environ["EMULATOR"]="true" before the display imports). Other docs I didn't touch (.cursor/plugin_templates/QUICK_START.md, docs/PLUGIN_DEVELOPMENT_GUIDE.md) already use the flag correctly. Replaced all 3 wrong statements with accurate guidance that both forms work and explains the CLI flag's relationship to the env var. .cursorrules, docs/GETTING_STARTED.md, docs/WEB_INTERFACE_GUIDE.md, docs/PLUGIN_DEVELOPMENT_GUIDE.md - Four places claimed "the plugin loader also falls back to plugins/". Verified that PluginManager.discover_plugins() (src/plugin_system/plugin_manager.py:154) only scans the configured directory — no fallback. The fallback to plugins/ exists only in two narrower places: store_manager.py:1700-1718 (store install/update/uninstall operations) and schema_manager.py:70-80 (schema lookup for the web UI form generator). Rewrote all four mentions with the precise scope. Added a recommendation to set plugin_system.plugins_directory to "plugins" for the smoothest dev workflow with dev_plugin_setup.sh symlinks. docs/FONT_MANAGER.md - The "Status" warning told plugin authors to use display_manager.font_manager.resolve_font(...) as a workaround for loading plugin fonts. Verified in src/font_manager.py that resolve_font() takes a family name, not a file path — so the workaround as written doesn't actually work. Rewrote to tell authors to load the font directly with PIL or freetype-py in their plugin. - The same section said "the user-facing font override system in the Fonts tab still works for any element that's been registered via register_manager_font()". Verified in web_interface/blueprints/api_v3.py:5404-5428 that /api/v3/fonts/overrides is a placeholder implementation that returns empty arrays and contains "would integrate with the actual font system" comments — the Fonts tab does not have functional integration with register_manager_font() or the override system. Removed the false claim and added an explicit note that the tab is a placeholder. docs/ADVANCED_FEATURES.md:523 - The on-demand section said REST/UI calls write a request "into the cache manager (display_on_demand_config key)". Wrong — verified via grep that api_v3.py:1622 and :1687 write to display_on_demand_request, and display_on_demand_config is only written by the controller during activation (display_controller.py:1195, cleared at :1221). Corrected the key name and added controller file:line references so future readers can verify. docs/ADVANCED_FEATURES.md:803 - "Plugins using the background service" paragraph listed all scoreboard plugins but an orphaned "⏳ MLB (baseball)" bullet remained below from the old version of the section. Removed the orphan and added "baseball/MLB" to the inline list for clarity. web_interface/README.md - The POST /api/v3/system/action action list was incomplete. Verified in web_interface/app.py:1383,1386 that enable_autostart and disable_autostart are valid actions. Added both. - The Plugin Store section was missing GET /api/v3/plugins/store/github-status (verified at api_v3.py:3296). Added it. - The SSE line-range reference was app.py:607-615 but line 619 contains the "Exempt SSE streams from CSRF and add rate limiting" block that's semantically part of the same feature. Extended the range to 607-619. docs/GETTING_STARTED.md - Rows/Columns step said "Columns: 64 or 96 (match your hardware)". The web UI's validation accepts any integer in 16-128. Clarified that 64 and 96 are the common bundled-hardware values but the valid range is wider. Not addressed (out of scope for docs PR): - .cursorrules:184 CodeRabbit comment flagged the non-existent cache_manager.delete() calls in src/common/api_helper.py:287 and src/plugin_system/resource_monitor.py:343. These are real CODE bugs, not doc bugs, and they're the first item in the planned post-docs-refresh code-cleanup PR (see /home/chuck/.claude/plans/warm-imagining-river.md). The docs in this PR correctly state that delete() doesn't exist on CacheManager — the fix belongs in the follow-up code PR that either adds a delete() shim or updates the two callers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -144,13 +144,21 @@ font = self.font_manager.resolve_font(
|
||||
> `"fonts"` block to your plugin's `manifest.json` will silently have
|
||||
> no effect — the FontManager method exists but nothing calls it.
|
||||
>
|
||||
> Until that's connected, plugin authors should ship custom fonts as
|
||||
> regular files inside the plugin directory (e.g., `assets/myfont.ttf`)
|
||||
> and reference them by relative path from the plugin's `manager.py`
|
||||
> via `display_manager.font_manager.resolve_font(...)` or by loading
|
||||
> with PIL directly. The user-facing font override system in the
|
||||
> **Fonts** tab still works for any element that's been registered via
|
||||
> `register_manager_font()`.
|
||||
> Until that's connected, plugin authors who need a custom font
|
||||
> should load it directly with PIL (or `freetype-py` for BDF) in
|
||||
> their plugin's `manager.py` — `FontManager.resolve_font(family=…,
|
||||
> size_px=…)` takes a **family name**, not a file path, so it can't
|
||||
> be used to pull a font from your plugin directory. The
|
||||
> `plugin://…` source URIs described below are only honored by
|
||||
> `register_plugin_fonts()` itself, which isn't wired up.
|
||||
>
|
||||
> The `/api/v3/fonts/overrides` endpoints and the **Fonts** tab in
|
||||
> the web UI are currently **placeholder implementations** — they
|
||||
> return empty arrays and contain "would integrate with the actual
|
||||
> font system" comments. Manually registered manager fonts do
|
||||
> **not** yet flow into that tab. If you need an override today,
|
||||
> load the font directly in your plugin and skip the
|
||||
> override system.
|
||||
|
||||
### Plugin Font Registration (planned)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user