-
Notifications
You must be signed in to change notification settings - Fork 33.9k
Snippets and Auto-closing pairs play nicer #114235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Snippets and Auto-closing pairs play nicer #114235
Conversation
@@ -43,11 +44,13 @@ suite('SnippetsService', function () { | |||
}); | |||
}); | |||
|
|||
let disposables: Array<IDisposable>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: We have a DisposableStore
which makes this a little simpler
teardown(function () { | ||
for (const disposable of disposables) { | ||
disposable.dispose(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would then be store.clear()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored this a bit so there's only one DisposableStore for the whole test suite and then I dispose it in suiteTeardown
In testing this snippet: "PSCustomObject": {
"prefix":"[PSCustomObject]",
"body":[
"[PSCustomObject] @{ Key = Value }"
]
} Here are my results with
So I am confused by the statement above: "Then, and only then will the eat the ] of the auto-closing pair and only if the user uses replace for the suggestion (SHIFT+ENTER by default) instead of the insert behavior of just ENTER." As you can see the end auto-closing pair was eaten for me using Enter and default I get the exact same results with
Am I missing something or is the description inaccurate? |
@ArturoDent are you sure your insertMode wasn't set to replace somewhere else? Because the default for Shift+Enter is replace....unless you set insertMode to replace in which case Shift+Enter does insert. |
Version: 1.53.0-insider (user setup) |
Look at the suggestion tooltip in your gif. It says "ENTER" for Replace and SHIFT+ENTER for Insert. Maybe you have a workspace setting? |
I have no workspace settings in that workspace. |
Here is a full list of results for a JSON file. One set with both user and workspace You see I tried both triggering the snippet with
Here are the results for a non-JSON file in fact a javascript file):
The second set of results is probably more in line with what you were expecting - although note there is no difference between I know JSON completion is trickier because what is treated as a word is different in those files. |
Using
and The only way I have found to actually |
Yes that all looks right. Keep in mind that "replace" behavior only decides if the content to the right of the cursor is included in the prefix on the left of the cursor. Everything to the left of the cursor is always replaced. |
This PR fixes #104952
Previously, auto-closing pairs would get in the way of some snippets if the snippet's prefix started with an autoclosing pair (like
[
). Here's an example:To fix/workaround this "odd behavior" (from an end user perspective) this PR uses the following logic to decide whether or not the snippet eats the character immidiately to the right of the cursor:
If this is satified, we say that the end of the snippet is endColumn++ thus eating the character to the right of the cursor.
This requires that the snippet more or less "opts-in" to this behavior by making sure that the prefix has autoclosing pairs around it... so if I wanted to opt-in the above snippet, I would change it to this:
Then, and only then will the eat the
]
of the auto-closing pair and only if the user uses replace for the suggestion (SHIFT+ENTER by default) instead of the insert behavior of just ENTER. If the user wants ENTER to have this behavior, they can apply the following setting:which is made exactly for this purpose.