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

Fix span used to determine whether signature help should be shown #436

Merged
merged 2 commits into from
Mar 21, 2016

Conversation

DustinCampbell
Copy link
Contributor

OmniSharp used the full span of an argument list to determine whether signature help should be shown. However, that includes both leading and trailing trivia, which means that it would return signatures for positions outside of the argument list. In addition, I switched it to use Contains(...) rather than IntersectsWith(...) since checking the position against the argument list span should not include the outer positions (e.g. on the left or right of the parentheses).

This fixes dotnet/vscode-csharp#64.

cc @david-driscoll, @troydai

@troydai
Copy link
Contributor

troydai commented Mar 20, 2016

:shipit:

@troydai
Copy link
Contributor

troydai commented Mar 20, 2016

Need rebase on latest dev. Travis build failures are fixed recently.

OmniSharp used the full span of an argument list to determine whether signature help should be shown. However, that includes both leading and trailing trivia, which means that it would return signatures for positions outside of the argument list. In addition, I switched it to use `Contains(...)` rather than `IntersectsWith(...)` since checking the position against the argument list span should not include the outer positions (e.g. on the left or right of the parentheses).
@DustinCampbell DustinCampbell force-pushed the fix-signature-help-span branch from 0614c5e to ee17829 Compare March 21, 2016 13:50
@DustinCampbell
Copy link
Contributor Author

rebased

@DustinCampbell
Copy link
Contributor Author

Cool. Passes now. Anyone else want to take a look?

@DustinCampbell
Copy link
Contributor Author

@david-driscoll?

@filipw
Copy link
Member

filipw commented Mar 21, 2016

This branch is out-of-date with the base branch
Merge the latest changes from dev into this branch

@DustinCampbell
Copy link
Contributor Author

@filipw -- huh? I totally just rebased against dev

@DustinCampbell
Copy link
Contributor Author

Trying again

@david-driscoll
Copy link
Member

@DustinCampbell that was me... I merge something else, but it shouldn't have been a conflict. Sorry. 🐑

@DustinCampbell
Copy link
Contributor Author

no worries. It really wasn't a conflict. 😄

@david-driscoll
Copy link
Member

No github is just a little more aggressive with it's merge policy.

filipw added a commit that referenced this pull request Mar 21, 2016
Fix span used to determine whether signature help should be shown
@filipw filipw merged commit 153a5e1 into OmniSharp:dev Mar 21, 2016
@filipw
Copy link
Member

filipw commented Mar 21, 2016

great 👍

@DustinCampbell DustinCampbell deleted the fix-signature-help-span branch September 22, 2016 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants