Fixing range for primary edit #59369
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
DanielRosenwasser
left a comment
DanielRosenwasser
left a comment
There was a problem hiding this comment.
There is another call to getText() below that is incorrectly used. It's arguable whether or not that should just reuse originalText, but I honestly am not familiar enough with the code.
But something must be wrong when copying/pasting trivia over, and the new test must not cover it.
I haven't found a case where the next |
| } | ||
| finally { | ||
| this.getScriptInfo(rootFile)?.editContent(0, this.program!.getSourceFile(rootFile)!.getText().length, originalText); | ||
| this.getScriptInfo(rootFile)?.editContent(0, updatedText.length, originalText); |
There was a problem hiding this comment.
Hmm... looking at it again, is it really safe to assume that cb doesn't itself change the contents of the file? I'm sure the way we use this function is used in our codebase, it's fine, but I just want to check. What do you think @sheetalkamat?
There was a problem hiding this comment.
yes we should assume cb doesnt change the contents of file since file is never "muted" without change in script info
There was a problem hiding this comment.
(assuming you mean mutated)
What I meant was that cb could itself run editContent somehow. If that happens, the ScriptInfo object may not actually contain the same text anymore.
I'm okay with assuming that will never happen for now, I just wanted to run it by you.
There was a problem hiding this comment.
The alternative is just to replace the text contents with the script info's own text contents. It should all be available anyway.
There was a problem hiding this comment.
Yes I did mean mutated
This reverts commit b04c8a0.
This reverts commit 0d5380b.
4 participants
Fixes #58936
Due to
getText()and notgetFullText(), new line characters were getting ignored when trying to perform a temporary file update which later made the paste at the wrong positions.