fix(lang-core): keep multi-line ternaries intact in mergeStatements - #866
Open
serhiizghama wants to merge 2 commits into
Open
fix(lang-core): keep multi-line ternaries intact in mergeStatements#866serhiizghama wants to merge 2 commits into
serhiizghama wants to merge 2 commits into
Conversation
splitStatementSource re-split the existing program with a char-level splitter that broke on every depth-0 newline, unlike split()/scanNewCompleted() which keep a ternary's ?/: continuation lines with the statement. So an untouched multi-line ternary lost both branches on merge. Track ternary depth and peek for a ? continuation, mirroring split().
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 #821.
A statement written as a multi-line ternary survives
parse(), butmergeStatements()was silently dropping both branches even when the patch never touched that statement:becomes
a = $okafter merging an unrelated patch.The cause is the third statement splitter.
split()in statements.ts andscanNewCompleted()in parser.ts both know that a?/:continuation line belongs to the same statement (they trackternaryDepthand peek past the newline), butmergeStatements()re-splits the existing program withsplitStatementSource()in merge.ts, which only tracked bracket depth and broke on every depth-0 newline. The? .../: ...fragments then failed theIdent = expressionshape check and were discarded without an error.Went with direction 1 from the issue: gave
splitStatementSource()the same ternary handling — track ternary depth at bracket depth 0, and before treating a depth-0 newline as a boundary, skip it if we are mid-ternary or the next meaningful character is a?. Didn't unify ontosplit()(direction 2) because merge needs the raw source slices to round-trip the statements, and the token splitter throws that text away.Added two tests next to the existing merge cases: the issue repro (untouched ternary stays intact) and the inverse (a patch targeting the ternary still replaces it). The first fails on the current splitter. Full lang-core suite is green, tsc/eslint/prettier clean.