redhat-developer / quarkus-ls

Language server for Quarkus tooling
Eclipse Public License 2.0
44 stars 15 forks source link

Qute completion returns both fields and accessors #929

Open fbricon opened 10 months ago

fbricon commented 10 months ago

That looks awkward:

Screenshot 2023-09-08 at 15 19 54 Screenshot 2023-09-08 at 15 27 36

@mkouba @ia3andy @fromage should completion show the accessors (e.g. defaultColumn() for a record, getName() for a class/interface)?

FroMage commented 10 months ago

That's a question for @mkouba but I suspect that the .name form will actually invoke both .getName() or .name() if it exists, making the longer forms not useful to complete.

mkouba commented 10 months ago

So for accessor methods it's possible to use the method name or the property name. For example, if a class Foo declares a method X getName() then in a template you can use {foo.name}, {foo.getName} or even {foo.getName()}. For boolean isActive() it's active, isActive and isActive(). For boolean hasFriends() it's friends, hasFriends and hasFriends(). Ofc the last variant with brackets is useless.

should completion show the accessors (e.g. defaultColumn() for a record,

I would recommned not to show the defaultColumn() variant.

getName() for a class/interface?

The same probably applies here, name is enough.

fbricon commented 10 months ago

@mkouba thanks, that's what I thought. Validation will still accept all possible variants, but completion should be less bloated

angelozerr commented 10 months ago

@fbricon I decided to show all possibilities in completion because when I read samples sometimes they were foo.name and sometimes foo.getName().

My idea was to provide a settings to show completion items .name or / and getName() or / and getName()

What do you think about that?

FroMage commented 9 months ago

Not sure I understand. But I agree the default should only show .name variants.

angelozerr commented 9 months ago

Not sure I understand. But I agree the default should only show .name variants.

I mean have a settings which drives the completion result to show or not .name / .getName() / .getName. By default the settings will be configured to show only .name

fbricon commented 9 months ago

@angelozerr I think that'd bring unnecessary complexity. I'd rather wait for some user to complain and request it

angelozerr commented 9 months ago

@angelozerr I think that'd bring unnecessary complexity. I'd rather wait for some user to complain and request it

Ok as it is just a filter matter, I think we could implement this setting internally and don't expose it to the user.