Fix column placeholders dropping the first selection row#11
Merged
Conversation
Column-format placeholders (`${cmd | cols N}`) always dropped the first
output row and shifted the remaining titles relative to their values.
In `resolveCmd` the title list was built with an unconditional `drop 1`
(to strip a header line), while the value list dropped a row only when
`hasHeader` was set. For a plain `cols` (no `+h`) the value list kept
every row but the title list was one shorter, so `zipWith` truncated to
the shorter list and the first row was lost. The surviving rows also had
their titles shifted by one relative to their values.
Extract the title/value pairing into a pure `formatColumns` helper that
only drops the header when `hasHeader` is set, and cover it with a
regression test.
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.
Problem
Running a command whose argument is a column-format placeholder, e.g.
### `git-worktree ${git-worktrees | cols 1}`drops the first row from the selection list. With three worktrees you only get two options, and the displayed titles are shifted by one relative to the values they select — so picking a row hands back the wrong path.
Cause
In
resolveCmd(src/Nixon/Command/Run.hs) the title list was built with an unconditionaldrop 1(intended to strip a header line), while the value list (parseColumns hasHeader) only dropped a row whenhasHeaderwas set:For a plain
cols(no+h,hasHeader = False) the values kept every row but the titles were one shorter, sozipWithtruncated to the shorter list — losing the first row and misaligning the rest. Withcols+hthe two happened to stay consistent, which is why only the headerless case was affected.Fix
Extract the title/value pairing into a pure, testable
formatColumnshelper inNixon.Formatthat only drops the header row whenhasHeaderis set, keeping titles and values aligned in both cases.Tests
Added regression tests in
Test.Nixon.Format.Columnscovering both the headerless case (the reported bug) and the+hcase. Verified the headerless test fails against the old behavior before the fix and passes after. Full suite: 126 examples, 0 failures.