Triggering autocompletion in comments if the next item is an exported member #1005 #1675
Conversation
|
|
src/goSuggest.ts
Outdated
| @@ -53,7 +53,15 @@ export class GoCompletionItemProvider implements vscode.CompletionItemProvider { | |||
| let autocompleteUnimportedPackages = config['autocompleteUnimportedPackages'] === true && !lineText.match(/^(\s)*(import|package)(\s)+/); | |||
|
|
|||
| if (lineText.match(/^\s*\/\//)) { | |||
| return resolve([]); | |||
| let nextLine = document.lineAt(position.line + 1).text; | |||
There was a problem hiding this comment.
If cursor is in the last line of the file, this will result in error
There was a problem hiding this comment.
Thanks for the feedback. I will add logic to check for this and include an test case.
src/goSuggest.ts
Outdated
| return resolve([]); | ||
| let nextLine = document.lineAt(position.line + 1).text; | ||
| let memberName = nextLine.replace(/\s+/g, ' ').trim().split(' '); | ||
| if (!nextLine.startsWith('\t')) { |
There was a problem hiding this comment.
I am guessing your assumption here is that the file is always formatted, which might not be the case. Instead look for func, var and type declarations in the nextLine.
There was a problem hiding this comment.
Will take these considerations into account and modify code.
src/goSuggest.ts
Outdated
| // check for exported members declared in block | ||
| memberName.shift(); | ||
| } | ||
| if (!memberName[0].match(/^[A-Z]/)) { |
There was a problem hiding this comment.
Say we found an exported member. In that case we should return a single item (the member name) in the suggestion list. Currently gocode gets called and it can return multiple results.
There was a problem hiding this comment.
In this case gocode should not be invoked right?
There was a problem hiding this comment.
Yes, we shouldn't invoke gocode. Instead we should return a single completion item with the member name right here
There was a problem hiding this comment.
Cool, will do.
|
Also, there have been conflicting changes in master. So first, merge from the master branch. |
|
Given the change and removal of |
There was a problem hiding this comment.
@shreyu86 The changes are good. I just have a few more suggestions. Please take a look
src/goSuggest.ts
Outdated
| // triggering completions in comments on exported members | ||
| if (position.line + 1 < document.lineCount) { | ||
| let nextLine = document.lineAt(position.line + 1).text.trim(); | ||
| let memberType = nextLine.match(/(const|func|type|var)\s*(\w+)/); |
There was a problem hiding this comment.
We need atleast one space between the keyword and member name, so \s* here should be s+
There was a problem hiding this comment.
Added the right regex.
| GreetingStatus = 1 | ||
| ) | ||
|
|
||
| // |
There was a problem hiding this comment.
Keep the cursor between the two / and manually trigger completions by pressing Ctrl+Space. We get the completion for SayHello, but we shouldn't
There was a problem hiding this comment.
Have added logic to handle this case.
src/goSuggest.ts
Outdated
| @@ -55,7 +55,22 @@ export class GoCompletionItemProvider implements vscode.CompletionItemProvider { | |||
| // prevent completion when typing in a line comment | |||
| const commentIndex = lineText.indexOf('//'); | |||
| if (commentIndex >= 0 && position.character > commentIndex) { | |||
| return resolve([]); | |||
| if (!lineText.trim().startsWith('//')) { | |||
There was a problem hiding this comment.
To avoid a lot of nesting, you can pull your code out and place it before line 55. You can start with
// triggering completions in comments on exported members
if (lineText.trim().startsWith('//') && (position.line + 1 < document.lineCount)) {
let nextLine = .....
}
src/goSuggest.ts
Outdated
| if (position.line + 1 < document.lineCount) { | ||
| let nextLine = document.lineAt(position.line + 1).text.trim(); | ||
| let memberType = nextLine.match(/(const|func|type|var)\s*(\w+)/); | ||
| if (memberType && memberType.length === 3) { |
There was a problem hiding this comment.
In the test file, at lines 11, 13, 18, 20 and 35 we get multiple completions from gocode and not from the code we specifically wrote for exported members.
This is because we don't exit with empty results when there is no match with the regex above.
There was a problem hiding this comment.
So in case of a block declaration, we should not trigger any completions and exit with empty results?
There was a problem hiding this comment.
Yes, unless you want to scan through the lines above and below the current line to look for the starting and ending of the block
There was a problem hiding this comment.
I am fine with skipping, wanted to clarify before I made any changes.
|
Thanks a lot! @ramya-rao-a |
|
Thanks for all the work @shreyu86! |
|
Thanks a lot for the refactor, I really appreciate the swift feedback. |
This pull request adds functionality described #1005 by triggering autocompletion in comments if the next line is an exported member. This would be handy in adding documentation for exported members.