fix(hello-world): centre the text, and always pass the configured colour - #380
Open
ChuckBuilds wants to merge 1 commit into
Open
fix(hello-world): centre the text, and always pass the configured colour#380ChuckBuilds wants to merge 1 commit into
ChuckBuilds wants to merge 1 commit into
Conversation
Two bugs in the plugin new plugins are copied from, which is what makes them worth more than their size. draw_text treats x as the LEFT edge unless centered=True is passed. The plugin passed x=width // 2 without it, under a comment reading "Calculate positions for centered text" -- so the default "Hello, World!" began at the midpoint of a 128-wide panel and ran off the right, rendering as "Hello, Worl". The default message did not fit the default panel. The harness could not catch it: PIL clips at draw time, so the drawn bounding box stays inside the panel and the overflow check sees nothing. check_plugin reports 8/8 PASS both before and after this change. It is only visible by looking at the output, which is what generating the README screenshots did. Separately, each draw_text call branched on whether the font manager returned a face, and the font-manager branch omitted color. On any install where a font manager is present -- which is the normal case on a Pi -- a configured colour was silently ignored and the text drew white. My own hash check passed the colours here only because the test harness supplies no font manager, so the else branch ran. The branches are collapsed: the face is chosen with `font=message_font or self.bdf_font` and the colour is passed either way, which also halves the code. The README documents all six settings with real renders and calls out both pitfalls in the template section, since a new plugin copied from here would otherwise inherit them. It also keeps the old file's examples, template file table and core doc links -- all four links verified to exist. Co-Authored-By: Claude Opus 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: Team 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 | 0 |
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.
Thirteenth plugin in the README pass — and two bugs in the plugin that new plugins are copied from, which is what makes them worth more than their size.
The default message did not fit the default panel
draw_texttreatsxas the left edge unlesscentered=Trueis passed. The plugin passedx=width // 2without it, under a comment reading "Calculate positions for centered text". So"Hello, World!"began at the midpoint of a 128-wide panel and ran off the right:Before —
Hello, Worl, starting at x=64:After — centred and complete. The hero image in the README is the fixed render.
The harness could not catch this. PIL clips at draw time, so the drawn bounding box stays inside the panel and the overflow check sees nothing —
check_plugin.pyreports 8/8 PASS both before and after. It was only visible by looking at the output, which is exactly what generating README screenshots does.A configured colour was silently ignored on real installs
Each
draw_textcall branched on whether the font manager returned a face, and the font-manager branch omittedcolor:On any install with a font manager — the normal case on a Pi — the message drew white regardless of
color.Worth noting how close I came to missing this: my own hash-check passed the colours, because the test harness supplies no font manager, so the
elsebranch ran. The check said "colour works" and it does, but only in the environment I was checking in. Reading the branch is what caught it.Both branches are collapsed —
font=message_font or self.bdf_fontwith the colour passed either way — which also halves the code.Why this one matters beyond itself
CLAUDE.mdsays new plugins start fromhello-world. Both mistakes are the kind that propagate silently into every plugin copied from it, so the README's template section now calls them out explicitly:draw_text(x=...)is the left edge — omitxto auto-centre, or passcentered=Truecoloron everydraw_textcall, never in only one branchAudits
check_plugin.py— 8/8 PASS;render_docs_assets --check— images match🤖 Generated with Claude Code