Include sortText in completions and improve suggestions

[pepeiborra]

Changes:

• Merge sort (by fuzzy score) all the sources of completions (module names, keywords, and identifiers). Previously they were individually sorted but never merged, leading to some poor completion choices
• Sort qualified completions up - previously some keywords could appear before
• Sort prefix completions up - e.g. head for hea over NoHeapProfiling
• Sort suggestions alphabetically
• Show the provenance in the detail field when no type info available

• Match the first character case-sensitively - e.g. don't suggest truncate for Tru, only True
• Fill in the sortText field to include our ordering (feature request: provide sortText with completionItems #2291). Previously the LSP client was sorting by label which was disrupting our ordering and breaking qualified completions in some cases (Autocompletion no longer counts qualified imports #2082)
• Extend the documentation attached to every identifier completion to state whether the identifier is already imported

Closes #2291 and #2082

[jneira]

LGTM thanks for improving this

[pepeiborra]

I made some more changes this morning after realising that alphabetical ordering is incompatible with qualified completions, so I have ditched the multi-dimensional sort key and gone back to a flat numbered sequence.

[michaelpj]

I made some more changes this morning after realising that alphabetical ordering is incompatible with qualified completions, so I have ditched the multi-dimensional sort key and gone back to a flat numbered sequence.

Is it incompatible? It seems to me like this means that there are three lexicographically ordered sorting factors:

1. local/nonlocal/global
2. qualified/unqualified (or maybe this is first? not sure)
3. name

We can encode all this into the sort text if we want: an integer for 1, and integer for 2, and then the text for 3.

(But I think I must be missing something, since it looks like you tried something like that already?)

[pepeiborra]

I made some more changes this morning after realising that alphabetical ordering is incompatible with qualified completions, so I have ditched the multi-dimensional sort key and gone back to a flat numbered sequence.

Is it incompatible? It seems to me like this means that there are three lexicographically ordered sorting factors:

1. local/nonlocal/global
2. qualified/unqualified (or maybe this is first? not sure)
3. name

We can encode all this into the sort text if we want: an integer for 1, and integer for 2, and then the text for 3.

(But I think I must be missing something, since it looks like you tried something like that already?)

Yes, this is true, we could extend the product with another factor to track qualified suggestions. It just gets a bit tedious to encode the triple in a string for the alphabetical ordering used by the client.

Basically, there are two ways that we can go about it:

a. Extend our sorting to cover all 3 (currently it's missing the last one, name) and then do a flat sequential ordering.
b. Encode all 3 factors in a string for the LSP client to sort alphabetically

It probably makes more sense to go with a, since we crop the list of suggestions by maxFactor before sending it, and we want to sort it optimally before cropping to ensure that we send the best suggestions.

[michaelpj]

It probably makes more sense to go with a, since we crop the list of suggestions by maxFactor before sending it, and we want to sort it optimally before cropping to ensure that we send the best suggestions.

Ooh, this is a great point, worth including in the comment. I think this alone basically means that the client can't realistically sort the results, since it doesn't have access to the full list. So we might as well just force our ordering.

[pepeiborra]

It probably makes more sense to go with a, since we crop the list of suggestions by maxFactor before sending it, and we want to sort it optimally before cropping to ensure that we send the best suggestions.

Ooh, this is a great point, worth including in the comment. I think this alone basically means that the client can't realistically sort the results, since it doesn't have access to the full list. So we might as well just force our ordering.

The client can sort, but if we don't sort then the quality of suggestions will be worse. I have implemented a few more improvements (see PR description) including alphabetical sorting but have not enabled alphabetical sorting since it breaks a lot of tests and I'm lazy

[tomsmeding]

This is awesome! Thanks for the work!

Match the first character case-sensitively - e.g. don't suggest truncate for Tru, only True

Is it reasonable to have this be configurable? I'd like to reduce my usage of the shift key, and I was very happy with the past behaviour where the whole match was case insensitive. If you consider this too much of a maintenance burden, that's fine too. :)