fix max-column-alignment-gap to demote anchor when gap limit exceeded#425
Open
sirmspencer wants to merge 1 commit into
Open
fix max-column-alignment-gap to demote anchor when gap limit exceeded#425sirmspencer wants to merge 1 commit into
sirmspencer wants to merge 1 commit into
Conversation
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.
@weavejester I can't remember if you want an issue per PR, so I included the details here. I can make it an issue if you prefer.
LLM notice
I have reviewed all of what is being submitted. The PR description was partially generated with llm to show the current and expected.
Problem
Here, I am not sure if this is a bug or a differene in expected behavior so I went with a new flag.
When
:max-column-alignment-gapis set, the alignment anchor is still the longest key in the form. Individual rows whose gap would exceed the limit fall back to a single space, but rows with medium-length keys are still extended all the way to the inflated anchor column. This produces inconsistent alignment: the shortest key falls back, medium keys over-extend, and the longest key (which caused the problem) sets the column for all of them.Current behavior
Config:
{:align-form-columns? true, :max-column-alignment-gap 5}Input:
Output —
long-namestays as anchor;afalls back (gap 9 > 5);abcdeandabcdefgextend unnecessarily to thelong-namecolumn:The same behavior affects maps with
:align-map-columns?.Solution
This PR does not change the behavior of
:max-column-alignment-gap. Instead it introduces a new flag,:max-column-anchor-gap, which applies the limit at anchor selection time rather than per row.When
:max-column-anchor-gap Nis set, cljfmt selects the largest key for which every shorter key would need at most N spaces to align. Keys longer than the chosen anchor are not padded.Expected behavior
Config:
{:align-form-columns? true, :max-column-anchor-gap 5}Composing both flags
The two flags are orthogonal and can be combined.
:max-column-anchor-gaprunsfirst to select a conservative anchor;
:max-column-alignment-gapthen appliesper-row fallback to the result.
Changes
:max-column-anchor-gapadded todefault-options(nil by default)column-start-positionapplies cascade demotion when:max-column-anchor-gapis setpad-to-positiongains a guard for keys wider than the demoted anchor, preventingtheir spacing from collapsing to zero
:max-column-alignment-gaptest-max-column-anchor-gapdeftest with form, map, and compose cases