Should completions be sorted by name?

[Techatrix]

Completions are sorted by the label since 1805837. 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:

zls/src/features/completions.zig

Lines 957 to 966 in 8cca7a1

	for (completions) |*c| {
	const prefix = kindToSortScore(c.kind.?) orelse continue;
	c.sortText = try std.fmt.allocPrint(arena, "{s}{s}", .{ prefix, c.label });
	if (source_index < source.len and source[source_index] == '(') {
	c.insertText = c.label;
	c.insertTextFormat = .PlainText;
	}
	}

[nektro]

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

[SuperAuguste]

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.

[btipling]

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.

[Techatrix]

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.

[btipling]

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.

[alichraghi]

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