zls icon indicating copy to clipboard operation
zls copied to clipboard

Should completions be sorted by name?

Open Techatrix opened this issue 1 year ago • 6 comments

Completions are sorted by the label since 1805837067cc68d563866cbf1eb5873dd64aa93a. Grouping them based on what kind (keyword, function, variable, ...) they are seems reasonable but sorted them alphabetically? You often group/order declarations in a way to make them easier to understand which gets undone by the sorting.

relevant code: https://github.com/zigtools/zls/blob/8cca7a1fa125494cd840569160c6373d41fce0b6/src/features/completions.zig#L957-L966

Techatrix avatar Feb 24 '24 04:02 Techatrix

imo they should be by 1) Leven distance from the test text or 2) source order of fields. on a personal note, ngl I got surprised by this because I thought editors took care of the ordering for you

nektro avatar Feb 24 '24 04:02 nektro

I agree that Levenshtein distance and order of fields could both be interesting metrics to try sorting by.

On editor handling ordering:

Editors do handle sorting for you as the spec says:

[...] the client is responsible for filtering and sorting. [...] However servers can enforce different behavior by setting a filterText / sortText

We override the sort text so we order certain types of completions above others. This gives us more control but means we have to implement more ourselves vs the editor implementing it for us.

SuperAuguste avatar Feb 24 '24 04:02 SuperAuguste

image I would expect entity here to appear before an import statement for a file I'm not currently using. I'm not sure if this is what is being discussed in this issue, but seemed similar, an ordering issue on a completion. I think it would save me a lot of time in aggregate, as I use entity a lot in my ECS based game and it always appears beneath the import. I'm also assuming this thing in my screenshot is ZLS related, sorry if it's not. I assumed it is because it's zig. I don't really know how vs code extensions work.

btipling avatar Mar 30 '24 04:03 btipling

I would expect entity here to appear before an import statement for a file I'm not currently using.

If the import statement is unused then why not remove it so it doesn't appear in the completions?

I do agree that entity should come first but for a different reason. I assume that in your code entity which is of type u64 can coerce to the second parameter type of has_id which is entity_t. This means that calling ecs.has_id(...,entity,...) is valid while ecs.has_id(...,entities,...) isn't so it becomes clear which one should be prioritized.

This is a far more complex (but better) mechanism than what this issue is about so created a separate issue in #1845.

Techatrix avatar Mar 30 '24 16:03 Techatrix

Yes entities is a file struct. and the argument to flecs entity_t is coerced from u64 in zig. You are correct that I should remove unused imports, but I often forget to, but I guess them appearing in completion should be a reminder! I have removed it. Thanks for creating the other issue I will follow that one too.

btipling avatar Mar 31 '24 23:03 btipling

a bit irrelevant but i think declarations should appear before keywords. the main reason is because currently allowzero is the first result when typing allo where you're typing for allocator. most keywords are also very short and barely need completion

alichraghi avatar Jul 14 '24 22:07 alichraghi