Add replacement span for string literal#37490
Add replacement span for string literal#37490DanielRosenwasser merged 6 commits intomicrosoft:masterfrom
Conversation
|
@typescript-bot pack this. |
|
Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build. |
| readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; | ||
| readonly allowTextChangesInNewFiles?: boolean; | ||
| readonly providePrefixAndSuffixTextForRename?: boolean; | ||
| readonly stringLiteralReplacementMode?: boolean; |
There was a problem hiding this comment.
@amcasey @uniqueiniquity @mjbvz do we really need this preference for compatibility given that the replacementSpan is existing property.
There was a problem hiding this comment.
Nope - if there's a replacementSpan, we'll handle it.
There was a problem hiding this comment.
Not my opinion, but I don't want to break anything
There was a problem hiding this comment.
we need to wait on @mjbvz to confirm if this is needed for vscode.. Otherwise this flag is not needed and shouldnt be added.
There was a problem hiding this comment.
Does the flag is added for the user who wants to keep the classic behavior, IMO...
I‘m not 100% sure everyone like the replacement.🤔
| if (!contextToken || !isStringLiteralLike(contextToken)) return undefined; | ||
| const entries = getStringLiteralCompletionEntries(sourceFile, contextToken, position, checker, options, host); | ||
| return convertStringLiteralCompletions(entries, sourceFile, checker, log, preferences); | ||
| const completeInfo = convertStringLiteralCompletions(entries, sourceFile, checker, log, preferences); |
There was a problem hiding this comment.
I would rather you do this inside the switch case of StringLiteralCompletionKind.Properties and StringLiteralCompletionKind.Types to ensure we aren't alternating any returned entries.
| readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; | ||
| readonly allowTextChangesInNewFiles?: boolean; | ||
| readonly providePrefixAndSuffixTextForRename?: boolean; | ||
| readonly stringLiteralReplacementMode?: boolean; |
There was a problem hiding this comment.
Also if this is really needed it shouldn't say node.. instead may be provideStringLiteralReplacementSpan
Kingwl
force-pushed
the
string_literal_replacement
branch
3 times, most recently
from
ae18871 to
36e65a5
Compare
Kingwl
force-pushed
the
string_literal_replacement
branch
from
36e65a5 to
1f9d8a7
Compare
|
This sounds safe to put in the 3.9 RC. What do you think @DanielRosenwasser ? |
|
@typescript-bot pack this |
|
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at b1a6127. You can monitor the build here. |
|
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running |
|
Looks like it's working! |
6 participants
Fixes #29270