feat(renderer): add left/right option for Default Padding#497
Open
vishnujayvel wants to merge 2 commits into
Open
feat(renderer): add left/right option for Default Padding#497vishnujayvel wants to merge 2 commits into
vishnujayvel wants to merge 2 commits into
Conversation
The Default Padding setting always applied to both sides of a widget. Add a defaultPaddingSide setting (both/left/right, default both) so padding can be narrowed to one side without affecting the other, matching the request in sirmalloc#482. Both the standard and Powerline renderers honor the new option, and alignment width accounting (calculateMaxWidthsFromPreRendered) counts only the sides that are actually applied. Existing configs are unaffected: defaultPaddingSide defaults to 'both', preserving current rendering exactly.
…TUI cycle Independent review found the padding-side tests asserted exact strings against chalk-wrapped padding output, which breaks under FORCE_COLOR=1 since chalk.reset() reacts to ambient color support, not the Settings colorLevel used elsewhere in the file. Strip SGR codes before every assertion, matching the repo's existing convention (stripSgrCodes in renderer-flex-width.test.ts and the separator-collapse tests). Verified passing under both FORCE_COLOR=1 and NO_COLOR=1. Also add the two coverage gaps called out in review: - merge:'no-padding' regression tests (standard + Powerline) proving no double-pad and correct glue across a no-padding merge boundary for both the 'left' and 'right' padding sides. - a GlobalOverridesMenu TUI test for the (d) cycle, mirroring the existing (m) minimalist-mode pattern.
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.
Closes #482, requested by @riptscripts.
Summary
The Default Padding setting always applied to both the left and
right of each widget. This adds a
defaultPaddingSidesetting(
both/left/right) so padding can be narrowed to one side.Existing configs are unaffected —
defaultPaddingSidedefaults to'both', so nothing changes unless you opt in.Where it's applied
renderStatusLineinsrc/utils/renderer.tsrenderPowerlineStatusLinein the same filecalculateMaxWidthsFromPreRenderednow counts only the side(s) actually applied, so Powerline
auto-align columns stay correct when padding is one-sided
merge: 'no-padding'interactions are preserved: a side that'salready suppressed by a no-padding merge stays suppressed regardless
of the padding-side setting. Covered by regression tests in both
renderers (see Testing below).
UI
Exposed in the Global Overrides menu (
src/tui/components/GlobalOverridesMenu.tsx),next to Default Padding:
Press (d) to cycle Both → Left only → Right only → Both. Available
in both standard and Powerline modes, matching how Default Padding
itself is already exposed in both modes.
Docs
Updated
docs/USAGE.md(Default Padding & Separators section) todescribe the new option and keybinding.
Testing
Added
src/utils/__tests__/renderer-padding-side.test.tscovering:both) behavior is unchangedleft/rightin standard renderingleft/rightin Powerline renderingcalculateMaxWidthsFromPreRenderedwidth accounting for each sidemerge: 'no-padding'takes precedence over the padding-side setting(no double-pad, correct glue) in both standard and Powerline mode,
for both
leftandrightAll assertions strip SGR codes before comparing (
stripSgrCodesfromsrc/utils/ansi.ts), per repo convention (seerenderer-flex-width.test.ts; theseparator-collapse tests strip SGR inline the same way) —
chalk.reset()reacts to ambientcolor-support (
FORCE_COLOR/NO_COLOR), independent of thecolorLevelset on
Settings, so an unstripped exact-string assertion would beenvironment-dependent. Verified the new file passing under both
FORCE_COLOR=1andNO_COLOR=1.Added a
GlobalOverridesMenu.test.tscase for the new(d)keybinding(cycles
both → left → right → both), mirroring the existing(m)minimalist-mode toggle test pattern.
Also updated two pre-existing widget tests
(
CurrentWorkingDir.test.ts,CustomCommand.test.ts) that constructa full
Settingsobject literal rather than spreadingDEFAULT_SETTINGS— they needed the new required field added, sameas any other defaulted
Settingsfield.Full-suite runs are noisy independent of this change: a handful of
Ink stdin-timing tests (e.g.
PowerlineThemeSelector,InstallMenu,occasionally a
GlobalOverridesMenugradient-selector case,fetchUsageData's error-handling timer case) flake under full-suiteparallel load but pass individually. Confirmed by running the full
suite twice on a clean, unmodified
maincheckout (viagit stash) —different tests flaked each time, none of them touching padding or
Settings. Every test file this PR touches passes consistently inisolation.
Notes
No
CURRENT_VERSION/migration bump needed —defaultPaddingSideisa new field with a Zod
.default('both'), the same pattern used forgitCacheTtlSeconds,minimalistMode, andexcludeFromAutoAlign(none of which bumped the version either).
🤖 Drafted with Claude Code; reviewed and submitted by a human.