[PATCH] D39738: [clangd] Sort completion results.

2017-11-09 Thread Francisco Lopes da Silva via Phabricator via cfe-commits
francisco.lopes added a comment.

OK. I do agree serialization, etc, is a stumbling block on delivering large 
results fast. I just wanted to warn on this because I've just seen ycmd devs 
went through this, where sorting was one of the bottlenecks, like, to 
`std::sort` 43k results instead of simply partial sorting them just over a 30 
results window, which is orders of magnitude faster.

If LSP completion ends up only feasible when there's filtering server side, 
with its own hard-coded sorting/filtering/ranking mechanisms, it's really sad, 
because it completely reduce flexibility of consumers wishing to apply their 
own (custom/configurable/uniform) sorting mechanisms.


Repository:
  rL LLVM

https://reviews.llvm.org/D39738



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39738: [clangd] Sort completion results.

2017-11-09 Thread Francisco Lopes da Silva via Phabricator via cfe-commits
francisco.lopes added a comment.

I'm still in the process of construction a general jsonrpc client and didn't 
start consuming clangd yet. But, I do have previous experience with libclang, 
and I don't know whether clangd will be able to return completion results at 
global scope (not necessarily on member access solely), like libclang does. 
This is my main concern, because global scope completion is one of the most 
useful use cases, even more in languages like C, but the results can be large 
and can go at the order of 43k results by simply including a header like 
windows.h . As I still 
didn't start consuming clangd, I don't know whether it even supports returns 
all the results from libclang to let the client do its filtering and sorting.


Repository:
  rL LLVM

https://reviews.llvm.org/D39738



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39738: [clangd] Sort completion results.

2017-11-09 Thread Francisco Lopes da Silva via Phabricator via cfe-commits
francisco.lopes added a comment.

Sorting got hardcoded? libclang has it optional. If hardcoded it becomes a 
performance problem, since some clients may wish to order it themselves with 
private heuristics.


Repository:
  rL LLVM

https://reviews.llvm.org/D39738



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38048: [clangd] Add textDocument/signatureHelp

2017-10-05 Thread Francisco Lopes da Silva via Phabricator via cfe-commits
francisco.lopes added a comment.

In https://reviews.llvm.org/D38048#889568, @rwols wrote:

> Since we're throwing around gifs for fun here's how signatureHelp looks in 
> practice for Sublime Text


Very cool!


https://reviews.llvm.org/D38048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38048: [clangd] Add textDocument/signatureHelp

2017-10-05 Thread Francisco Lopes da Silva via Phabricator via cfe-commits
francisco.lopes added a comment.

In https://reviews.llvm.org/D38048#889176, @ilya-biryukov wrote:

> @francisco.lopes, thanks for sharing your experience, I definitely like the 
> final UX you achieved, we should aim for something similar in clangd.
>  It also looks like @rwols's  idea of how things should look like is very 
> close to your implementation.


Nice!

> I do think that showing signatures for non-overloaded functions in completion 
> may still be useful. Overloaded functions can be grouped by name and include 
> some indicators that there are more overloads (something like `foo(int)   
> void  (+5 overloads)`.
>  How do you choose return type for the completion item if function is 
> overloaded? Show it only when it's the same for all overloads? Go with one of 
> the return types?

In fact it didn't happen to me to think about the return information in the 
popup menu at the time, so it was not changed!, and it should just show one 
return type from one of the overloads randomly (I must never look at return 
types...).
I do think the (+ 5 overloads) approach is a good one too, though I prefer to 
not oversize the menu width with parameters.

It will be cool if clangd and its Language Server Protocol implementation 
manages to have feature parity with this, personally I'd be looking in 
migrating from YouCompleteMe to a LSP solution that's more modular and 
lightweight, possibly bringing to it
what gets implemented here myself.


https://reviews.llvm.org/D38048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38048: [clangd] Add textDocument/signatureHelp

2017-10-04 Thread Francisco Lopes da Silva via Phabricator via cfe-commits
francisco.lopes added a comment.

In https://reviews.llvm.org/D38048#887960, @ilya-biryukov wrote:

> Another case where this might be bad is overloaded functions. I may choose 
> one overload in completion, but `signatureHelp` will initially point into a 
> different one.


Hi,  I'd just like to comment on this aspect to inform how  this is done is 
different ways for Vim.

Original YouCompleteMe, for example, still doesn't support parameter hints, so 
it just offers function prototypes in the completion menu, for functions 
foo(int, char) and
foo(void *, int), the two prototypes gets listed and not mattering what 
function the user selects the text to be inserted will always just be the same 
function name, foo.

I didn't find this useful because it cluttered the menu with many overloads 
that the user may not be in fact looking for, it may be targeting for another 
function but it gets
like ten long overloads of another function that's ranked first according to 
what the user typed.

I also didn't find it useful, in Vim, to show full prototypes in  the pop up 
menu because for C++ they tend to be rather long.

So I changed YouCompleteMe this way:

- Just show unique function names in the popup menu (they're tagged as 
functions).
- When the user selects a given function, the overloads are shown in a separate 
preview window instead, which for me is more sane for showing long information.
- When the user goes about filling the call site with arguments, the overloads 
in the preview window are dynamically updated showing the current argument the 
user is at, for the remaining compatible overloads.
- The overloads get reduced while the user fill arguments as well as there's 
type resolution of generic functions along the way.

These pictures show how this happens:

- https://s3.amazonaws.com/f.cl.ly/items/1e2F0A123h331c1G0L0R/SadBart.gif
- https://imgur.com/a/OqFaI

They just don't happen illustrate the reduction of multiple overloads because I 
didn't put multiple ones at the time.

This is just to illustrate some different completion approach that tries to be 
inline with libclang offerings.


https://reviews.llvm.org/D38048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D21113: Add support for case-insensitive header lookup

2016-12-19 Thread Francisco Lopes da Silva via Phabricator via cfe-commits
francisco.lopes added a comment.

I'm waiting for this to land as well, it's crucial for coding windows code with 
libclang assistance on a linux machine.


https://reviews.llvm.org/D21113



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits