Only refuse partial ready --fix files that formatting changes#17
Merged
Conversation
Summary
Stop `sley ready --fix --commit` from blocking commits of partially
staged files whose staged content is already formatter-clean.
The commit gate refused every partially staged file (both staged and
unstaged hunks) upfront, to avoid folding formatter output across the
unstaged hunk. But that also blocked the common "stage some hunks, keep
editing, then commit" workflow even when nothing needed formatting — a
regression the dotfiles `githook-test` guards against ("partial-stage
clean content: commit succeeds").
Decide per file by result instead of refusing upfront:
- Pass 1 probes partially staged files without mutating them: back the
worktree copy up, format, then always restore it. If formatting would
have changed the file, record it; a clean partial file is a pure no-op.
If the backup can't be made, refuse rather than risk an unrecoverable
clobber.
- If any partial file would be reformatted, refuse before touching the
rest of the batch, so a refusal never leaves other files reformatted in
the worktree.
- Pass 2 formats every non-partial file in place as before, reporting the
re-stage requirement when the formatter changes a fully staged file.
The standalone `sley fix` command still refuses partial files outright;
the commit gate is deliberately more permissive so clean partial-stage
commits succeed.
Testing
- update the `ready --fix --commit` partial-staging tests: a partial file
that the formatter WOULD change is still refused (exit 2) with the
worktree left intact (the probe reverts); a mixed batch refuses before
the fully staged file is ever formatted
- add a regression test mirroring the commit hook: a clean partial-staged
file with a no-op formatter is NOT refused, the worktree is left intact,
and the run reaches exit 0
- verified the dotfiles `githook-test` passes against this build
(`PATH=<worktree>/bin` → 51 passed, 0 failed), including the two
partial-stage assertions that fail on the previous behavior
- full suite green (`./test/sley-test`: ok); `checkrun lint` and
`shellcheck -x lib/sley/ready.sh` clean
Summary Address two review findings on the partial-staged probe. - the restore `cp -p` return code was unchecked: if the restore failed after the formatter had already rewritten the worktree file (disk full, file turned read-only), execution fell through and deleted the only backup — the exact clobber this guard prevents. Now check the restore, and on failure keep the backup and refuse loudly, naming both paths. - redirect the formatter's stdin from /dev/null in both passes so a formatter that reads stdin can't drain the herestring driving the loop, matching the standalone `_sley_fix` guard. Testing - full suite green (`./test/sley-test`: ok); `checkrun lint` and `shellcheck -x lib/sley/ready.sh` clean - dotfiles `githook-test` still passes against this build (51/0)
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.
Summary
Stop
sley ready --fix --commitfrom blocking commits of partiallystaged files whose staged content is already formatter-clean.
The commit gate refused every partially staged file (both staged and
unstaged hunks) upfront, to avoid folding formatter output across the
unstaged hunk. But that also blocked the common "stage some hunks, keep
editing, then commit" workflow even when nothing needed formatting — a
regression the dotfiles
githook-testguards against ("partial-stageclean content: commit succeeds").
Decide per file by result instead of refusing upfront:
worktree copy up, format, then always restore it. If formatting would
have changed the file, record it; a clean partial file is a pure no-op.
If the backup can't be made, refuse rather than risk an unrecoverable
clobber.
rest of the batch, so a refusal never leaves other files reformatted in
the worktree.
re-stage requirement when the formatter changes a fully staged file.
The standalone
sley fixcommand still refuses partial files outright;the commit gate is deliberately more permissive so clean partial-stage
commits succeed.
Testing
ready --fix --commitpartial-staging tests: a partial filethat the formatter WOULD change is still refused (exit 2) with the
worktree left intact (the probe reverts); a mixed batch refuses before
the fully staged file is ever formatted
file with a no-op formatter is NOT refused, the worktree is left intact,
and the run reaches exit 0
githook-testpasses against this build(
PATH=<worktree>/bin→ 51 passed, 0 failed), including the twopartial-stage assertions that fail on the previous behavior
./test/sley-test: ok);checkrun lintandshellcheck -x lib/sley/ready.shclean