Fix trailing formatting edit when range ends mid-token #50082
Merged
Conversation
This file contains 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
typescript-bot
added
Author: Team
For Milestone Bug
andrewbranch
commented
Jul 28, 2022
| // with a selection from the beginning of the line to the space inside the curly braces, | ||
| // inclusive. We would expect a format-selection would delete the space (if rules apply), | ||
| // but in order to do that, we need to process the pair ["{", "}"], but we stopped processing | ||
| // just before getting there. This block handles this trailing edit. |
There was a problem hiding this comment.
This comment should have been included with #46875.
| // produce bogus edits if we try to `processPair`. Recall that the point of this logic is | ||
| // to perform a trailing edit at the end of the selection range: but there can be no valid | ||
| // edit in the middle of a token where the range ended, so if we have a non-contiguous | ||
| // pair here, we're already done and we can ignore it. |
There was a problem hiding this comment.
This comment describes the fix for this bug.
jakebailey
approved these changes
Jul 28, 2022
|
Yeah, it’s correlated with the existence of |
Comment on lines
+451
to
+452
| // but in order to do that, we need to process the pair ["{", "}"], but we stopped processing | ||
| // just before getting there. This block handles this trailing edit. |
There was a problem hiding this comment.
Suggested change
| // but in order to do that, we need to process the pair ["{", "}"], but we stopped processing | |
| // just before getting there. This block handles this trailing edit. | |
| // but in order to do that, we need to process the pair ["{", "}"]. | |
| // This block handles this trailing edit by ensuring we continue processing. | |
| // the pair of tokens that the range intersects. |
| const tokenInfo = | ||
| formattingScanner.isOnEOF() ? formattingScanner.readEOFTokenRange() : | ||
| formattingScanner.isOnToken() ? formattingScanner.readTokenInfo(enclosingNode).token : | ||
| undefined; | ||
|
|
||
| if (tokenInfo) { | ||
| if (tokenInfo && tokenInfo.pos === previousRangeTriviaEnd!) { | ||
| // We need to check that tokenInfo and previousRange are contiguous: the `originalRange` |
There was a problem hiding this comment.
Suggested change
| // We need to check that tokenInfo and previousRange are contiguous: the `originalRange` | |
| // We need to check that previousRange and tokenInfo are contiguous since the `originalRange` |
DanielRosenwasser
approved these changes
Jul 28, 2022
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.
Fixes #50076