Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Fix updating detailed completion for overloaded methods #2633

Merged

Conversation

feltech
Copy link
Contributor

@feltech feltech commented Oct 14, 2018

  • For languages that allow method overloads, where the same method name
    can be defined with different arguments (e.g. C++), the label of a
    completion may be duplicated in the completions list, but the detail
    should be different.
  • Hence use the detail field, if available, to disambiguate between
    completion items when updating them with the result of a
    completionItem/resolve.

* For languages that allow method overloads, where the same method name
can be defined with different arguments (e.g. C++), the `label` of a
completion may be duplicated in the completions list, but the `detail`
should be different.
* Hence use the `detail` field, if available, to disambiguate between
completion items when updating them with the result of a
`completionItem/resolve`.
@feltech
Copy link
Contributor Author

feltech commented Oct 14, 2018

An example from cquery (C++) completions

Before the fix - both completion items appear to be duplicates:
oni_completion_detail_1
oni_completion_detail_2

With the fix the second "duplicate" item shows it's different signature:
oni_completion_detail_fixed

@codecov
Copy link

codecov bot commented Oct 14, 2018

Codecov Report

Merging #2633 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2633      +/-   ##
==========================================
+ Coverage   45.33%   45.38%   +0.04%     
==========================================
  Files         361      361              
  Lines       14572    14576       +4     
  Branches     1913     1915       +2     
==========================================
+ Hits         6606     6615       +9     
+ Misses       7741     7737       -4     
+ Partials      225      224       -1
Impacted Files Coverage Δ
browser/src/Services/Completion/CompletionStore.ts 73.42% <100%> (+4.36%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca6a4d1...98084c8. Read the comment docs.

Copy link
Member

@akinsho akinsho left a comment

Choose a reason for hiding this comment

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

Nice spot 👍thanks for the fix

@akinsho akinsho merged commit fdc740c into onivim:master Oct 14, 2018
@feltech feltech deleted the bug/completion_resolve_splats_overloads branch October 14, 2018 21:31
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