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

each on a collection is not enough promoted on content assist #674

Open
mauromol opened this issue Aug 14, 2018 · 3 comments
Open

each on a collection is not enough promoted on content assist #674

mauromol opened this issue Aug 14, 2018 · 3 comments

Comments

@mauromol
Copy link

mauromol commented Aug 14, 2018

Somewhat related to #411.

Consider the following code:

new List().eac|

Invoke content assist at "|": it's quite clear and obvious that I want to invoke one of the each methods supplied as DGMs, but Greclipse as a first option shows forEach method.
I understand that content assist priority is currently determined first by group, than by name matching (giving more priority to class methods rather than DGMs), but I personally think that prefix and exact matches should ALWAYS be given maximum priority. It's even more evident when you type new List().each| and invoke content assist, it's quite natural that Greclipse should suggest each before forEach.

Also, the implementation detail on how DGMs are provided by Groovy I think should be, as said, an implementation detail to the Groovy developer: from a developer (I mean, consumer of Greclipse) point of view, the fact that each is not defined on List but rather as a DGM should not matter in any way. It could be reasonable to give DGMs less relevance (like you do for methods deeply inherited in class hierarchy) than methods directly defined in the declared type, but IMHO prefix vs substring matching should be the first priority factor when deciding what to show first.

After all, if I want to type each (and I know I want to type each) it's annoying that Greclipse suggests me that I want to type forEach instead, especially when you have content assist auto activation and you then type a trigger character.
Also consider that, in this particular case, each, even though defined as a DGM for Object, fits perfectly and naturally on collection classes and in this way Greclipse makes it "hard" to use.

@mauromol
Copy link
Author

Another example in which the current proposal order is not useful, combined with the problem already described in #676:

package test31;

import java.beans.PropertyChangeListener;

public class A {

	public final PropertyChangeListener[] getPropertyChangeListeners() {
		return null;
	}
	
	public boolean isBar() {
		return true;
	}
}
package test31

class Test31 {

	private static final A FAKE = new A()
	
	void foo() {
		def b = new A()
		if(b.is|)
	}
}

Suppose you're at "|" and you want to type: b.is(FAKE) (i.e.: is operator). When you type it one character by one, starting from "b", you'll get: b.propertyChangeListeners( (as soon as you'll type the "(" character).
This is because auto-activation automatically selects the first proposal (see #676). But if the first proposal were is, instead of propertyChangeListeners, it would have ended correctly!

Also note that propertyChangeListeners comes before isBar().
IMHO the correct suggestion order should be:

  1. is (exact match)
  2. isBar (prefix match, target class)
  3. isCase (prefix match, DGM)
  4. propertyChangeListeners (substring match, property-style access)
  5. getPropertyChangeListeners (substring match, getter access)
  6. registerNatives (private static on Object - not sure what it is and if it should be shown... isn't it private?)

@eric-milles
Copy link
Member

Note for anyone looking at this and wanting to alter the sort order of content assist proposals in either the Groovy or Java editor:

You can implement the extension point javaCompletionProposalSorters to provide your own custom sorting order. This will appear as an option in Window > Preferences > Java > Editor > Content Assist > Sort proposals (drop down). It is similar to implementing java.util.Comparator and there are two reference implementations linked in the above documentation.

sort-proposals

Groovy content assist merely provides relevance values to achieve the default sorting.

@mauromol
Copy link
Author

mauromol commented Aug 23, 2019

Any plan to work on this? I found another frequent use case in which working with Greclipse right now is painful.
Suppose you're using slf4j (a quite common logging framework) and you want to type:

logger.debug| <= insert "(" here

Greclipse completes it as: logger.debugEnabled(, which is twice wrong because:

  • my typing clearly matches debug (as in org.slf4j.Logger.debug(...)) more than debugEnabled: once again, I strongly think that exact matching should have the priority over prefix matching
  • the use of "(" quite clearly indicates the desire to perform a method call, while debugEnabled is a property

The second point might be tricky (I don't know if you can discriminate over the trigger character based on context), but if the first point were fixed the second problem would probably be negligible.

Right now, if you have auto-activation enabled you are doomed!!

Please, consider changing the content assist priorities based on the type of matching (in order of priority: exact, prefix, substring, camel-case...). IMHO it would be an impressive improvement in Greclipse usability, really.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants