Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Avoid duplicate function parameter body on function suggest snippets #1696

Merged
merged 3 commits into from
Jun 3, 2018

Conversation

lggomez
Copy link
Contributor

@lggomez lggomez commented May 29, 2018

Avoid adding snippet for function suggest when cursor is followed by (). Fixes #1655

Avoid adding snippet for function suggest when cursor is followed by (). Fixes microsoft#1655
src/goSuggest.ts Outdated
let newSnippetString = null;
// Avoid adding snippet for function suggest when cursor is followed by ()
// i.e: met() -> method()()
if ((nextSymbolIndex < lineText.length) && (lineText.charAt(nextSymbolIndex) === '(')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not check lineText.substr(position.character, 2) === '()' instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

src/goSuggest.ts Outdated
// i.e: met() -> method()()
if ((nextSymbolIndex < lineText.length) && (lineText.charAt(nextSymbolIndex) === '(')) {
newSnippetString = new vscode.SnippetString(suggest.name);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not skip setting the item.insertText altogether if the cursor is followed by ()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because keeping the text insert will still autocomplete the function name sans the param snippets

src/goSuggest.ts Outdated
// Avoid adding snippet for function suggest when cursor is followed by ()
// i.e: met() -> method()()
if (lineText.substr(position.character, 2) === '()') {
newSnippetString = new vscode.SnippetString(suggest.name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lggomez

We don't need to create a vscode.SnippetString in this case.
When item.insertText is not set, then item.label is used by the auto-completion feature.
See https://github.com/Microsoft/vscode/blob/1.23.0/src/vs/vscode.d.ts#L3047-L3052

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ramya-rao-a ramya-rao-a merged commit 10acfbd into microsoft:master Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants