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

Don`t process function signatures on comment lines #2496

Merged
merged 7 commits into from
May 7, 2019
Merged

Don`t process function signatures on comment lines #2496

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c249f49
Merge remote-tracking branch 'upstream/master'
lggomez Apr 21, 2019
7501734
Merge remote-tracking branch 'origin/master'
lggomez May 3, 2019
f87a5fc
Don´t process function signatures on comment lines
lggomez May 5, 2019
c4e886a
Remove false positives on comment line check
lggomez May 5, 2019
4617bed
Fix regression on parameter backtracking
lggomez May 7, 2019
5b7adc7
Improve comment detection
lggomez May 7, 2019
76543e3
Remove old comment
ramya-rao-a May 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/goSignature.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ export class GoSignatureHelpProvider implements SignatureHelpProvider {
for (let lineNr = position.line; lineNr >= 0 && maxLookupLines >= 0; lineNr-- , maxLookupLines--) {

const line = document.lineAt(lineNr);

// Stop processing if the line is a comment
if (line.text.trim().indexOf('//') === 0) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will only catch the case where the entire line starts with the comment.
What about the below case?

x, err := regexp.Match("abc", data) // I have some comment here,

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 is the most basic case of course. Either way, I'm noticing the bug isn't completely dependent on comments presence so there is still something else in it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to reuse the same function made for suggestions logic on this one

}

// if its current line, get the text until the position given, otherwise get the full line.
const [currentLine, characterPosition] = lineNr === position.line
? [line.text.substring(0, position.character), position.character]
Expand Down