fix(hook): stop four shapes from closing a live block - #151
Merged
Conversation
The command-position approximation read a reserved word that is not a
command as one, popped a block that was still open, and the body was
then rewritten: a consumer of the block read snip's compacted output
instead of the real one. All four corrupt identically on master.
- A case-arm pattern is a pattern, never a command. blockScope now
tracks the arm state of a case, entered at `in` and re-entered at each
`;;`, so `case $s in done)` no longer pops the case. `esac` still
closes it, since bash rejects a bare `esac` as a pattern.
- Inside a paren that is not a subshell, '|' is pattern alternation, so
it no longer restores command position: `for f in @(data|done).txt`
keeps its loop open.
- `time` keeps command position across its own `-p` and `--` options, so
the opener it times is still seen.
- A '#' flush against an operator ')' opens a comment, which isWordStart
cannot tell from the ')' of a command substitution. advance decides it
where the kind of the paren is known, leaving the segmenter's own
isWordStart untouched (a '{' there would break ${#var}).
Verified end to end against real bash with a 60-line file behind
filters/grep.yaml's cap of 50: all four now return 60 where they
returned 51.
Refs #138
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.
Closes the first four shapes of #138. The fifth (a trailing backslash line continuation) has a different cause and is left for its own PR.
The defect
The command-position approximation read a reserved word that is not a command as one, popped a block that was still open, and the body was then rewritten. A consumer of the block read snip's compacted output instead of the real one. All four corrupt identically on master.
Reproduced end to end against real bash, 60-line file behind
filters/grep.yaml's cap ofhead n:50:case $s in done)for f in @(data|done).txttime -p for i in 1a)# done markerThe fixes
blockScopetracks the arm state of a case: entered at theinthat ends the subject, re-entered at each;;, left at the arm's). Words read in that state are patterns, sodone)no longer pops the case.esacis the one exception, because bash rejects a bareesacas a pattern; without it the scope would stay open and everything after it would lose filtering.;;is reported by the segmenter, since ';' is a group boundary andadvancenever sees one.timeoptions.-pand--keep command position, so the opener being timed is still seen. Baretimeis unchanged.)#. A '#' flush against an operator ')' opens a comment.advancedecides this where the kind of the paren being closed is known, which distinguishes it from$(ls)#tailwhere the ')' ends a command substitution and the '#' is still inside the word. The segmenter's ownisWordStartis deliberately untouched: adding '{' there would read${#var}as a comment.The file-level comment listing these four as known limitations is updated in the same commit.
Tests
TestRewriteCommandPositioncovers the four shapes, the;;re-arm on a later arm, and five non-regressions:$(ls)#tail,${#f}, a subshell)#, baretime, and filtering resuming afteresac. Every case fails on master.make test-race,make verify(55/55) andgolangci-lintare clean.