Skip to content

fix single-column-line? to detect first element in collection#424

Merged
weavejester merged 1 commit into
weavejester:masterfrom
sirmspencer:ms-fix-single-column-first-element
Jul 26, 2026
Merged

fix single-column-line? to detect first element in collection#424
weavejester merged 1 commit into
weavejester:masterfrom
sirmspencer:ms-fix-single-column-first-element

Conversation

@sirmspencer

@sirmspencer sirmspencer commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

@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. The test cases were partially generated with llm to cover all the permutations.

Problem

:align-single-column-lines? false does not suppress anchor inflation when the wrapped form is the first binding in a let or the first entry in a map. The first-position element is always treated as an alignment anchor, producing the same excessive padding as :align-single-column-lines? true.

The underlying cause is in single-column-line?, which identifies whether a node is the sole element on its line. It checks for a linebreak to the left of the node, but the first element in a collection has no left sibling — z/left* returns nil, line-break? of nil returns false, and the element is never recognized as single-column regardless of what is on its right.

Current behavior

Form columns

Config: {:align-form-columns? true, :align-single-column-lines? false}

Input:

(let [{:keys [endpoint mapper]}
      (get-in config [:auth])
      token (get response "access_token")
      type (get response "token_type")]
  {:token token})

Output — first-position wrapped binding used as anchor despite false:

(let [{:keys [endpoint mapper]}
      (get-in config [:auth])
      token                     (get response "access_token")
      type                      (get response "token_type")]
  {:token token})

Map columns

Config: {:align-map-columns? true, :align-single-column-lines? false}

Input:

{:very-long-map-key
 wrapped-value
 :host "localhost"
 :port 5432
 :timeout 30}

Output — first-position entry used as anchor despite false:

{:very-long-map-key
 wrapped-value
 :host              "localhost"
 :port              5432
 :timeout           30}

Expected behavior

Form columns

;; first-position wrapped binding excluded; token and type align locally
(let [{:keys [endpoint mapper]}
      (get-in config [:auth])
      token (get response "access_token")
      type  (get response "token_type")]
  {:token token})

Map columns

;; first-position entry excluded; :host/:port align to :timeout
{:very-long-map-key
 wrapped-value
 :host    "localhost"
 :port    5432
 :timeout 30}

Fix

single-column-line? currently requires a linebreak to the left:

(line-break? (skip-whitespace-and-commas (z/left* zloc) z/left*))

A nil result (first element in collection) is treated as false. The fix treats nil
the same as a linebreak:

(let [left (skip-whitespace-and-commas (z/left* zloc) z/left*)]
  (or (nil? left) (line-break? left)))

Tests cover all three positions (first, middle, last) for both align-form-columns?
and align-map-columns?, with both true and false flag values, added to the
existing test-align-single-column-lines-option deftest.

@weavejester weavejester left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Can you change the commit message so it adheres to the seven rules of a great Git commit message as per the contributing guidelines. I also have a suggestion regarding the use of some-> that I've added as a comment.

Comment thread cljfmt/src/cljfmt/core.cljc
Comment thread cljfmt/test/cljfmt/core_test.cljc Outdated
@sirmspencer
sirmspencer force-pushed the ms-fix-single-column-first-element branch 4 times, most recently from bc005ea to 161c125 Compare July 26, 2026 16:14
Extend the left-side check to treat a nil left sibling as equivalent
to a linebreak, so first-position nodes in a collection are correctly
identified as single-column and excluded from the alignment anchor
when :align-single-column-lines? is false.
@sirmspencer
sirmspencer force-pushed the ms-fix-single-column-first-element branch from 161c125 to 5820b66 Compare July 26, 2026 16:21
@sirmspencer

sirmspencer commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@weavejester Sorry, I forgot the commit and line width rules. They are updated. for the comment on some->

the some-> form is not symantically the same. when z/right* or z/left* returns nil (nothing there, or first element), some-> returns nil which is falsy, but nil should be a passing condition. The form below works as well. This change copied the existing logic block, but either is fine with me.

(defn- single-column-line? [zloc]
  (let [no-break? (complement line-break?)]
    (not (or (some-> zloc z/right* (skip-whitespace-and-commas z/right*) no-break?)
             (some-> zloc z/left*  (skip-whitespace-and-commas z/left*)  no-break?)))))

@weavejester

Copy link
Copy Markdown
Owner

Ah, right. In which case, keep it the way it is. I think that's clearest.

@weavejester
weavejester merged commit 23b08c9 into weavejester:master Jul 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants