Review Request 115446: Further kdev-clang code completion improvements.

[Sergey Kalinichev]

> On Feb. 3, 2014, 5:15 p.m., Milian Wolff wrote:
> > codecompletion/context.cpp, line 249
> > <https://git.reviewboard.kde.org/r/115446/diff/1/?file=241520#file241520line249>
> >
> >     Instead of removing this, I initially wanted to make use of the KTextEditor::TemplateInterface, which you can see in action in the snippets. That would allow us to quickly navigate between the placeholders, replace them and continue adding code after the function. 
> >     
> >     Anyhow, I don't think you have to do that now, so I'm OK with disabling this for now. But please add a comment at least here.

I see. Also I've just tried that KTextEditor::TemplateInterface feature and dunno it seems very weird for me... For example after insertTemplateText inserted f(int i). "int i" is not selectable all together i.e. you can select either "int" or "i"; Also parameters can be very huge (especially when it comes to templates) so it's very difficult to navigate through/edit it.
  Anyhow, maybe I just don't completely understand how it's suppose to work, so yes I'll insert a comment saying that we should consider KTextEditor::TemplateInterface possibility.


> On Feb. 3, 2014, 5:15 p.m., Milian Wolff wrote:
> > codecompletion/context.cpp, line 297
> > <https://git.reviewboard.kde.org/r/115446/diff/1/?file=241520#file241520line297>
> >
> >     while this does help showing the overloaded items, it does so in a very costly way (repeatedly looking up the qid declarations). So rather, I suggest keeping the existing qid-based QSet but adding items for all overloaded items at once.

Are you sure this is an issue, especially when there is usually no more than 3-5 overloaded methods as compared to ~100 usual, so the performance penalty IMO is not so big +/-10 ms.
  Otherwise adding all overloaded items at once will significantly complicate the code (as all items will have the same display value and we somehow have to retrieve the correct ones...).
So I suggest we leave it that way and if time shows that it's really a hot spot we could come up with a better solution then.


> On Feb. 3, 2014, 5:15 p.m., Milian Wolff wrote:
> > codecompletion/context.cpp, line 305
> > <https://git.reviewboard.kde.org/r/115446/diff/1/?file=241520#file241520line305>
> >
> >     this should instead use ClassDeclaration::isPublicBaseClass and its argument to find the depth. Or maybe even found->context()->depth() ?

Well, nothing of that solves the problem.
Here is what I wanted to achieve by doing this:
snippet:  int i; void f(int j); f()
Pressing crt+space inside of f() will suggest a lot of global items and "i" variable somewhere in the middle of it, to prevent this I wanted to check that a declaration belongs to the current file, if so move it to the top of completion list.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115446/#review48824
-----------------------------------------------------------


On Feb. 3, 2014, 4:25 p.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115446/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2014, 4:25 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> *Don't insert arguments of a function into text on execute(..)
> *Append () to functions 
> *Properly (I hope...) handle overloaded functions.
> *Show local items above others.
> 
> P.S: Now, when I've got commit rights, should I still post these patches for review or not?
> 
> 
> Diffs
> -----
> 
>   codecompletion/context.cpp 6c4df1c 
> 
> Diff: https://git.reviewboard.kde.org/r/115446/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140204/c7ef173e/attachment.html>