Fix: ASCII renderer inverted by default#692
Closed
MJenius wants to merge 2 commits intoShane32:masterfrom
Closed
Conversation
added 2 commits
March 9, 2026 16:12
Contributor
📝 WalkthroughWalkthroughThis change corrects the inverted rendering in the ASCII 'small' renderer by reassigning the palette character mappings in the GetGraphicSmall method. The Unicode block characters and space are swapped to properly represent white and black module color combinations. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@QRCoder/ASCIIQRCode.cs`:
- Around line 112-115: Tests still expect the old palette ordering so update the
small-renderer fixtures in AsciiQRCodeRendererTests to match the new constants
(WHITE_ALL, WHITE_BLACK, BLACK_WHITE, BLACK_ALL) by swapping/rewriting the
expected outputs: swap the default-case expected output with the invert: true
case (or otherwise invert the fixtures) so the filled quiet zone and
inverted-render expectations align with the new palette mapping; modify the
assertions in QRCoderTests/AsciiQRCodeRendererTests.cs (the test method(s)
around AsciiQRCodeRendererTests) to use the corrected expected strings for the
default and invert:true cases and keep the test names/structure unchanged.
- Around line 112-115: Default inversion for the public helper
AsciiQRCodeHelper.GetQRCode(..., bool invert = true) must be flipped to false to
match the palette change; locate the GetQRCode method declaration and change its
default parameter to invert = false (and update any other overloads or callers
that rely on the old default), ensuring the new default aligns with the ASCII
palette constants (WHITE_ALL, WHITE_BLACK, BLACK_WHITE, BLACK_ALL) so
GetGraphicSmall(invert: false) and the helper's default behavior produce the
same non-inverted output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e7c979e-f6dd-4ff0-ae03-ba20d59403fb
⛔ Files ignored due to path filters (15)
QRCoderDemoUWP/Assets/LockScreenLogo.scale-200.pngis excluded by!**/*.pngQRCoderDemoUWP/Assets/SplashScreen.scale-200.pngis excluded by!**/*.pngQRCoderDemoUWP/Assets/Square150x150Logo.scale-200.pngis excluded by!**/*.pngQRCoderDemoUWP/Assets/Square44x44Logo.scale-200.pngis excluded by!**/*.pngQRCoderDemoUWP/Assets/Square44x44Logo.targetsize-24_altform-unplated.pngis excluded by!**/*.pngQRCoderDemoUWP/Assets/StoreLogo.pngis excluded by!**/*.pngQRCoderDemoUWP/Assets/Wide310x150Logo.scale-200.pngis excluded by!**/*.pngQRCoderTests/PdfByteQRCodeRendererTests.can_render_pdfbyte_qrcode_blackwhite.approved.pdfis excluded by!**/*.pdfQRCoderTests/PdfByteQRCodeRendererTests.can_render_pdfbyte_qrcode_color.approved.pdfis excluded by!**/*.pdfQRCoderTests/PdfByteQRCodeRendererTests.can_render_pdfbyte_qrcode_custom_dpi.approved.pdfis excluded by!**/*.pdfQRCoderTests/PdfByteQRCodeRendererTests.can_render_pdfbyte_qrcode_from_helper.approved.pdfis excluded by!**/*.pdfQRCoderTests/PdfByteQRCodeRendererTests.can_render_pdfbyte_qrcode_from_helper_2.approved.pdfis excluded by!**/*.pdfQRCoderTests/TransposeVerificationTests.pdf_renderer.approved.pdfis excluded by!**/*.pdfQRCoderTests/assets/noun_software engineer_2909346.pngis excluded by!**/*.pngnuget-icon.pngis excluded by!**/*.png
📒 Files selected for processing (1)
QRCoder/ASCIIQRCode.cs
Comment on lines
+112
to
+115
| WHITE_ALL = " ", | ||
| WHITE_BLACK = "\u2584", | ||
| BLACK_WHITE = "\u2580", | ||
| BLACK_ALL = "\u2588", |
Contributor
There was a problem hiding this comment.
Update the small-renderer fixtures in AsciiQRCodeRendererTests.
QRCoderTests/AsciiQRCodeRendererTests.cs:38-75 still hardcodes the pre-fix output: the default case expects the filled quiet zone, and the invert: true case expects the non-inverted one. After this palette swap, those expectations need to be swapped/rewritten as well, otherwise the corrected behavior is either untested or the existing tests will fail.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@QRCoder/ASCIIQRCode.cs` around lines 112 - 115, Tests still expect the old
palette ordering so update the small-renderer fixtures in
AsciiQRCodeRendererTests to match the new constants (WHITE_ALL, WHITE_BLACK,
BLACK_WHITE, BLACK_ALL) by swapping/rewriting the expected outputs: swap the
default-case expected output with the invert: true case (or otherwise invert the
fixtures) so the filled quiet zone and inverted-render expectations align with
the new palette mapping; modify the assertions in
QRCoderTests/AsciiQRCodeRendererTests.cs (the test method(s) around
AsciiQRCodeRendererTests) to use the corrected expected strings for the default
and invert:true cases and keep the test names/structure unchanged.
Flip the helper overload default to false too.
This palette swap corrects GetGraphicSmall(invert: false), but Line 197 still declares AsciiQRCodeHelper.GetQRCode(..., bool invert = true). Calls through that public helper will therefore keep returning the inverted output by default, so the bug remains on one entry point.
Suggested follow-up
- public static string GetQRCode(string plainText, ECCLevel eccLevel, bool forceUtf8 = false, bool utf8BOM = false, EciMode eciMode = EciMode.Default, int requestedVersion = -1, string endOfLine = "\n", bool drawQuietZones = true, bool invert = true)
+ public static string GetQRCode(string plainText, ECCLevel eccLevel, bool forceUtf8 = false, bool utf8BOM = false, EciMode eciMode = EciMode.Default, int requestedVersion = -1, string endOfLine = "\n", bool drawQuietZones = true, bool invert = false)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@QRCoder/ASCIIQRCode.cs` around lines 112 - 115, Default inversion for the
public helper AsciiQRCodeHelper.GetQRCode(..., bool invert = true) must be
flipped to false to match the palette change; locate the GetQRCode method
declaration and change its default parameter to invert = false (and update any
other overloads or callers that rely on the old default), ensuring the new
default aligns with the ASCII palette constants (WHITE_ALL, WHITE_BLACK,
BLACK_WHITE, BLACK_ALL) so GetGraphicSmall(invert: false) and the helper's
default behavior produce the same non-inverted output.
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
1 participant
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.
Fixes #646
Summary by CodeRabbit