feat: implement box ol and box ul ol decoration styles - #2215
Open
VXNCXNX wants to merge 1 commit into
Open
Conversation
Both variants existed in DecorationStyle but fell through to a plain box behind a TODO, so the overline whisker was silently dropped. Parameterize the top-right corner of write_boxed_partial so it can be a whiskered joint, and wire the two arms to it. box ol is now the vertical mirror of box ul, and box ul ol carries a whisker on both edges. The two pre-written tests for this are no longer ignored, and box ul ol gets equivalent coverage. Fixes dandavison#214.
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.
Fixes #214.
What was missing
0d593c2made decoration styles compose, butbox olandbox ul olwere left as TODOs that fell through to a plain box:So
--commit-decoration-style='box ol'silently rendered the same thing asbox, with no error to tell you the style wasn't supported.The fix
write_boxed_partialhardcoded┐as the top-right corner. It now takes anoverline: boolthat selects┐or┬─, and that flag threads up throughwrite_boxed_with_horizontal_whisker. The four public draw functions become thin wrappers over two shared bodies, sobox/box olandbox ul/box ul oldiffer only in that flag rather than in duplicated drawing code.Result —
box olis the vertical mirror ofbox ul:Tests
You'd already written the expected output for both cases as
#[ignore]d tests (test_commit_style_box_ol,test_file_style_box_ol). Those are the spec here — I implemented against them and removed the#[ignore], without touching a single assertion string or width argument.box ul olhad no pre-written test, so I added two modeled exactly on theolpair.What I checked beyond the tests
boxandbox ulat widths 64/10/1/variable, plus raw and bold modes, before and after the change: 12/12 byte-identical.box olnever touchesline_width;box ul olreuses the existing guardedline_width - box_width - 1. Ran all four styles at--width=10and--width=1against a 46-char commit line, exit 0, no panic.DOWN_HORIZONTALis destructured in the sameis_boldbranch as the other glyphs, so it can't desync. Bold renders┳.--commit-style rawstill works with both new styles.One thing worth your call: the overline whisker is a fixed 2 chars (
┬─) regardless of terminal width, sincebox-family functions ignoreline_width. That's what the ignored test asserts, and it matches the existing_line_width: &Width, // ignoredprecedent, but if you'd rather the overline whisker extend to fill the width like the underline one does, that's a small follow-up.