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

Fix prevention of completion popup in line comments. (fixes #1659) #1671

Merged
merged 3 commits into from
May 13, 2018

Conversation

doxxx
Copy link
Contributor

@doxxx doxxx commented May 12, 2018

Fixes #1659 by checking for a line comment at the end of the current line and also checking that the cursor position is within the line comment.

src/goSuggest.ts Outdated
@@ -52,7 +52,9 @@ export class GoCompletionItemProvider implements vscode.CompletionItemProvider {
let lineTillCurrentPosition = lineText.substr(0, position.character);
let autocompleteUnimportedPackages = config['autocompleteUnimportedPackages'] === true && !lineText.match(/^(\s)*(import|package)(\s)+/);

if (lineText.match(/^\s*\/\//)) {
// prevent completion when typing in a line comment
const commentMatch = lineText.match(/\/\/.*$/);
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 just find the index directly using lineText.indexOf('//') ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.... That's a very good point. :)

src/goSuggest.ts Outdated
if (lineText.match(/^\s*\/\//)) {
// prevent completion when typing in a line comment
const commentIndex = lineText.indexOf('//');
if (commentIndex > 0 && position.character > commentIndex) {
Copy link
Contributor

Choose a reason for hiding this comment

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

commentIndex being 0 is valid. This check should be commentIndex > -1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good catch!

@ramya-rao-a ramya-rao-a merged commit 0673847 into microsoft:master May 13, 2018
@doxxx doxxx deleted the fix-1659 branch October 31, 2018 14:18
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