Skip to content
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

insertSnippet can put snippet selections in wrong positions #145727

Closed
MrAndersen1 opened this issue Mar 22, 2022 · 7 comments · Fixed by #146179
Closed

insertSnippet can put snippet selections in wrong positions #145727

MrAndersen1 opened this issue Mar 22, 2022 · 7 comments · Fixed by #146179
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders snippets verified Verification succeeded
Milestone

Comments

@MrAndersen1
Copy link

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.66.0-insider (Universal) Commit: db0525f
  • OS Version: macOS Big Sur 11.6

Steps to Reproduce:
I created a new extension to test this where I basically use a keybinding to trigger a command inserting a snippet.

Speed is of the essence in a lightweight environment/extension in order to reproduce this so that's why we need the keybinding.

"contributes": {
	"commands": [
		{
			"command": "testextension.helloWorld",
			"title": "Hello World"
		}
	],
	"keybindings":[
		{
			"command": "testextension.helloWorld",
			"key": "ctrl+m"
		}
	]
}
vscode.commands.registerCommand('testextension.helloWorld', async () => {
	const line = vscode.window.activeTextEditor?.document.lineAt(vscode.window.activeTextEditor?.selection.start);
	const snippet = new vscode.SnippetString("aProperty: aClass<${2:boolean}> = new aClass<${2:boolean}>();\n");
	// I believe, but am not sure, that it's important that the place to insert the snippet on differs from the cursor position.
	await vscode.window.activeTextEditor?.insertSnippet(snippet, new vscode.Position(line?.lineNumber! + 5, 4));	
});

in a new file containing

{
    text
    more text
    some more text
    even more text
    and some more
    additional
    text
}

putting the cursor after the first "text", pressing enter twice quickly followed by crtl+m puts the snippet cursors on "ean> = " and "ean>();" instead of "boolean" and "boolean"

@jrieken jrieken added this to the April 2022 milestone Mar 25, 2022
@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Mar 25, 2022
@jrieken
Copy link
Member

jrieken commented Mar 25, 2022

I can reproduce but don't know yet what's happening...

@jrieken
Copy link
Member

jrieken commented Mar 25, 2022

This happens because there is an indent only line. Somehow, it is being cleared when/while the snippet edit is being made. That invalidates the offset we have cached at the start of the snippet session. The four whitespace characters that are being removed are exactly those four character by which the placeholders are shifting.

From the recording you can see that this only happens when the indentation is cleaned-up (line 4) while the snippet is being inserted.

Screen.Recording.2022-03-25.at.17.12.27.mov

@jrieken
Copy link
Member

jrieken commented Mar 25, 2022

/cc @alexdima let's discuss Monday. It seems that I am suffering from some auto-clean on edit behaviour here

@MrAndersen1
Copy link
Author

Nice, will test it when I get my hands on it.

@jrieken jrieken added the author-verification-requested Issues potentially verifiable by issue author label Mar 28, 2022
@MrAndersen1
Copy link
Author

It works fine on my end now @jrieken 👍

@MrAndersen1
Copy link
Author

/verified

@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders snippets verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@jrieken @MrAndersen1 @alexdima and others