Sorting completion candidates based on clang priority

[434189275]

Sorting completion candidates based on clang priority

[Sarcasm]

Hello and thank you for proposing this change!

Candidates are currently sorted according to clang_sortCodeCompletionResults(), to benefit from the performances of letting libclang doing it. And personally, I prefer the predictability of alphabetical ordering.

Some frontends may like to sort them that way, e.g. when merging candidates from multiple backends.

I do expose the priority to elisp, so that the frontend (e.g. company-irony) can sort accordingly. So, if you use the company frontend, I would suggest you to make a patch there, and enable it only under a defcustom.

[434189275]

Hello and very nice mode by the way! Keep up the good work!

I personally use it mainly for C++ with company-irony, and the reason why I submitted this pull request is that sorting according to libclang priority (clang_getCompletionPriority()) allows completions to be context-aware, and thus, more relevant. Take for example this situation:

In that case, I think most users would expect to have the sprite variable in first position, sprintf being less relevant here.

Now, sorting according to libclang priority:

sprite variable now in first position, results still being sorted alphabetically.

And personally, I prefer the predictability of alphabetical ordering.

I totally agree with you on that point, but I think you'll agree that alphabetical sorting should not prevail on the relevance of the completion.

As you see, I could have just removed the call to clang_sortCodeCompletionResults() to have results sorted according to priority only, but as I, too, want the results to be sorted alphabetically, I manually sort them after calling clang_sortCodeCompletionResults() so we have both the benefits of the clang priority and of the alphabetical sorting.

What do you think ?

[Sarcasm]

I do think it's a legitimate use case, but I think it can be done in company-irony, in a defcustom variable, so that it's easier to enable/disable than something in the C++ part.

I believe you can wrap this around company-irony--candidates:

https://github.com/Sarcasm/company-irony/blob/52aca45bcd0f2cb0648fcafa2bbb4f8ad4b2fee7/company-irony.el#L78

Do you see what I mean?

[434189275]

I get what you mean, but I definitely think that sorting should not be done in the front end for obvious performance reasons. More over, it can very easily be disabled by the mean of a defcustom variable, passed to the candidates command along with the prefix and style arguments.

Now, I do not see the need for disabling it, as totally ignoring clang priority actually leads up to incorrect completions in my opinion. And as I said above, alphabetical sorting is not put apart, it is just improved by taking into account the clang priority.

[Sarcasm]

Sorting in C++ could be faster, but sorting in company, assuming the default prefix length of size 3, does not sort all the candidates, but only a subset. So in the general case, I'm not sure of how slower it would be, could really depend on the prefix.

Ok for prefix and style-like method, I would accept such PR. I reopen if you prefer this solution.