Skip to content
This repository has been archived by the owner on Aug 3, 2024. It is now read-only.

Autocomplete relevance #474

Closed
wants to merge 1 commit into from
Closed

Autocomplete relevance #474

wants to merge 1 commit into from

Conversation

ocozalp
Copy link
Contributor

@ocozalp ocozalp commented Jul 6, 2016

Relevance parameter has been taken into account when sorting code
completion proposals.

Relevance parameter has been taken into account when sorting code
completion proposals.
@@ -91,7 +91,6 @@ public void accept(CompletionProposal proposal)
case CompletionProposal.LOCAL_VARIABLE_REF:
case CompletionProposal.VARIABLE_DECLARATION:
case CompletionProposal.ANNOTATION_ATTRIBUTE_REF:
case CompletionProposal.POTENTIAL_METHOD_DECLARATION:
Copy link
Owner

Choose a reason for hiding this comment

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

What's the reasoning behind this removal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of the

if (proposal.getKind() != CompletionProposal.POTENTIAL_METHOD_DECLARATION)

statement, this case is useless.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, I was just looking at the diff. Makes sense after looking beyond that.

@@ -55,6 +55,10 @@ public int compare(CodeCompleteResult o1, CodeCompleteResult o2)
return -1;
}

if (o1.getRelevance() != o2.getRelevance()) {
return o2.getRelevance() - o1.getRelevance();
Copy link
Owner

Choose a reason for hiding this comment

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

minor nitpick, but eclim code uses 2 spaces for indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. I will change my editor settings.

@@ -185,6 +184,9 @@ protected CodeCompleteResult createCompletionResult(
// of whether the user ever views it.
/*return new CodeCompleteResult(
kind, completion, menu, proposal.getAdditionalProposalInfo());*/
return new CodeCompleteResult(completion, menu, menu, type, offset);
CodeCompleteResult result = new CodeCompleteResult(completion, menu, menu, type, offset);
result.setRelevance(proposal.getRelevance());
Copy link
Owner

Choose a reason for hiding this comment

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

Sorting the completions by relevance should be an option (new command argument) since this will have an adverse affect on vim's code completion mechanism which, unfortunately, works best with completions that are sorted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my experience with eclim, it is hard to find an item in autocomplete if it is in the same package. With this option, autocomplete order becomes the same with Eclipse and it is easier to use IMHO.

Copy link
Owner

@ervandew ervandew Nov 28, 2016

Choose a reason for hiding this comment

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

Can you provide an example? I'm not saying that there shouldn't be an option to sort by relevance, but it should be just that, an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will provide some screenshots as soon as possible.

@@ -66,7 +66,7 @@ public Object execute(final CommandLine commandLine)
if(COMPACT.equals(layout) && results.size() > 0){
results = compact(results);
}
Collections.sort(results);
Copy link
Owner

Choose a reason for hiding this comment

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

Did you intend to affect completion for all eclim supported languages here or were you hoping to just affect the java completions? This breaks quite a few unit tests.

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 was trying to change for Java. If it breaks tests, I will re-add the line.

lukroth added a commit to lukroth/eclim that referenced this pull request Jan 5, 2017
- To enable the eclim client to sort the proposals according to their relevance the relevance field is set.
- The relevance field is not passed along when using compact mode -> fixed.

This commit is a subset of the commit made for 'Autocomplete relevance ervandew#474'.
lukroth added a commit to lukroth/eclim that referenced this pull request Jan 11, 2017
- To enable the eclim client to sort the proposals according to their relevance the relevance field is set.
- The relevance field is not passed along when using compact mode -> fixed.

This commit is a subset of the commit made for 'Autocomplete relevance ervandew#474'.
@ocozalp ocozalp closed this Jul 12, 2024
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