Skip to content

Conversation

kermorgant
Copy link
Contributor

@kermorgant kermorgant commented Jul 22, 2018

Follows reflextions posted here

annotation is directly taken from phpactor's info key

image

I've just written this and have already consumed too much time :-) So this PR probably deserves a review and a bit of testing before merging ;-)

annotation is directly taken from phpactor's ``info`` key
@kermorgant
Copy link
Contributor Author

I can already say there's an issue when one candidate can match several Fully Qualified Names (FQN).

if you try to complete Request, you'll see only one candidate for "Request" but the FQN in the annotation can change during completion.

this removes the call to add-to-list which eliminated duplicates
(interactive (company-begin-backend 'company-phpactor))
(prefix (company-phpactor--grab-symbol))
(candidates (all-completions (substring-no-properties arg) (mapcar #'(lambda (suggestion) (plist-get suggestion :name)) (company-phpactor--get-suggestions))))))
(candidates (all-completions arg (company-phpactor--get-candidates)))))
Copy link
Contributor Author

@kermorgant kermorgant Jul 22, 2018

Choose a reason for hiding this comment

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

I have the impression calling all-completions is useless. Am I wrong about that ?
@MarTango I believe you pushed that, do you remember if there was some use for it ?

Copy link
Contributor

Choose a reason for hiding this comment

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

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, no worry. After reading the doc of all-completions, I understand it filters the candidates given in the second argument by ensuring each one begins with the first one. This seems to be redundant with what phpactor already does.

@kermorgant
Copy link
Contributor Author

Given the PR about auto-import contains the same changes, I guess this one should probably be closed.

@kermorgant
Copy link
Contributor Author

Closing this one after merging pr #34

@kermorgant kermorgant closed this Jul 25, 2018
@kermorgant kermorgant deleted the completion-annotation branch September 16, 2018 11:56
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.

2 participants