Refuse partially staged files in sley ready --fix#16
Merged
Conversation
Summary Make the `sley ready --fix` git path refuse partially staged files, so the commit gate shares one formatting-safety policy with the standalone `sley fix` command. `sley ready --fix --commit` is the real commit gate — the Git/Sapling commit hook runs it on every commit-producing command. Its git `--fix` path formatted each selected file in place with no guard against a partially staged file (one with both staged and unstaged hunks). Formatting rewrites the whole worktree file, folding the formatter's output into the unstaged hunk — the exact corruption `sley fix` already refuses via `_sley_git_staged_partial_files`. The two paths had divergent, unsafe-vs-safe policy. - add the guard to the git `--fix` branch, gated on the staged/commit scope (`_SLEY_SCOPE_CHANGE == staged`) — the only scope with an index to corrupt; plain `ready --fix` and non-git repos are unaffected, matching `sley fix` - filter the selected files through `_repo_existing_regular_files` before the partial check, exactly as `sley fix` does, so symlinks and non-regular files (which the format loop skips anyway) can't trigger a spurious refusal - on a partial file, print the same "refusing files with staged and unstaged changes" message, drop the selected-files cache, and return 2, byte-for-byte matching `sley fix` - refusing partials first also makes the existing "formatter modified staged files" report accurate: every remaining file then has worktree content equal to its index, so the loop's worktree-hash comparison no longer mis-handles a file whose worktree differs from the index Testing - new `sley-behavior-test` cases: `ready --fix --commit` refuses a single partial file (exit 2, formats nothing, worktree unchanged); refuses a mixed batch where any file is partial (refuse-all parity, clean file left unformatted); and still formats a fully-staged clean file (exit 1 with the re-stage report) so the guard does not over-refuse - verified red-green: neutering the scope gate fails the refusal cases (formatter runs, worktree corrupted) while the clean-file case stays green - full suite green (`./test/sley-test`: ok); `checkrun lint` and `shellcheck -x lib/sley/ready.sh` clean
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
Make the
sley ready --fixgit path refuse partially staged files, sothe commit gate shares one formatting-safety policy with the standalone
sley fixcommand.sley ready --fix --commitis the real commit gate — the Git/Saplingcommit hook runs it on every commit-producing command. Its git
--fixpath formatted each selected file in place with no guard against a
partially staged file (one with both staged and unstaged hunks).
Formatting rewrites the whole worktree file, folding the formatter's
output into the unstaged hunk — the exact corruption
sley fixalreadyrefuses via
_sley_git_staged_partial_files. The two paths haddivergent, unsafe-vs-safe policy.
--fixbranch, gated on the staged/commitscope (
_SLEY_SCOPE_CHANGE == staged) — the only scope with an indexto corrupt; plain
ready --fixand non-git repos are unaffected, matchingsley fix_repo_existing_regular_filesbeforethe partial check, exactly as
sley fixdoes, so symlinks andnon-regular files (which the format loop skips anyway) can't trigger a
spurious refusal
unstaged changes" message, drop the selected-files cache, and return 2,
byte-for-byte matching
sley fixstaged files" report accurate: every remaining file then has worktree
content equal to its index, so the loop's worktree-hash comparison no
longer mis-handles a file whose worktree differs from the index
Testing
sley-behavior-testcases:ready --fix --commitrefuses a singlepartial file (exit 2, formats nothing, worktree unchanged); refuses a
mixed batch where any file is partial (refuse-all parity, clean file
left unformatted); and still formats a fully-staged clean file (exit 1
with the re-stage report) so the guard does not over-refuse
(formatter runs, worktree corrupted) while the clean-file case stays
green
./test/sley-test: ok);checkrun lintandshellcheck -x lib/sley/ready.shclean