Case insensitive Identifiers

[David Nolden]

Am Montag 15 Februar 2010 16:27:18 schrieb Niko Sams:
> On Mon, Feb 15, 2010 at 16:00, David Nolden <zwabel at googlemail.com> wrote:
> > Am Montag 15 Februar 2010 14:50:34 schrieb Niko Sams:
> >> On Mon, Feb 15, 2010 at 13:13, David Nolden <zwabel at googlemail.com> 
wrote:
> >> > Am Montag 15 Februar 2010 12:49:50 schrieb Niko Sams:
> >> >> Hi all (David),
> >> >>
> >> >> In Php class identifiers and function identifiers as case
> >> >> insensitive. Currently there are two problems
> >> >> with that:
> >> >> - QuickOpen displays the tolower version
> >> >> - I want to change code completion to use CodeModel only (for
> >> >> performance) and i'ts tolower then too
> >> >>
> >> >> two ideas how to solve this:
> >> >> 1. add a CaseInsensitive flag to findDeclarations
> >> >
> >> > That's not possible, because the global PersistentSymbolTable does not
> >> > support case-insensitive look-up, and it would probably be a major
> >> > amount of work to make that work.
> >> >
> >> >> 2. when showing QuickOpen/CodeCompletion items load shown
> >> >> declarations
> >> >
> >> > At least for quick-open, this shouldn't be a problem, as it happens
> >> > anyway! Quickopen loads all the declarations that are currently
> >> > visible completely, so it can disambiguate multiple overloads of the
> >> > same function, and such stuff. You'd just have to modify it so it also
> >> > extract the non-tolower name when loading the declarations. I think
> >> > the place to do this is
> >> > DUChainItemData::text.
> >>
> >> Ah, ok this patch would fix the issue:
> >> --- a/kdevplatform/plugins/quickopen/duchainitemquickopen.cpp
> >> +++ b/kdevplatform/plugins/quickopen/duchainitemquickopen.cpp
> >> @@ -46,7 +46,7 @@ QString DUChainItemData::text() const {
> >>        if(def->declaration())
> >>            decl = def->declaration();
> >>
> >> -  QString text = decl->qualifiedIdentifier().toString();
> >> +  QString text = decl->toString();
> >
> > I think that won't work, because we need the whole qualified identifier,
> > and toString() only returns the local identifier (we need
> > 'Namespace::Name' and not just 'Name')
> 
> Ah, ok, Php doesn't have namespaces :D
> Alternative solutions?

But you also have "ClassName::functionName", no? There you'll need it as well. 
So you probably have to traverse the scope somehow.

You could also implement a "virtual QString 
Declaration::prettyQualifiedIdentifier() const { return 
qualifiedIdentifier().toString() }" and override it in your php stuff to 
return the non-tolower version.

> >>    if(!decl->abstractType()) {
> >>
> >> Is is Ok?
> >>
> >> >> This is not easily possible with CodeCompeltion as kate uses the
> >> >> NameColumn and DisplayRole for sorting
> >> >>
> >> >>
> >> >> Any ideas how to solve this isue?
> >> >
> >> > Hmm in code-completion it's more problematic, that's true. So you
> >> > don't want to use declarations any more at all? At least in C++, the
> >> > declarations are used anyway so they can be filtered and such, and in
> >> > that case it wouldn't be a problem to also extract the non-tolower
> >> > name at that point.
> >>
> >> I do want to use them, but they should lazy load - as currently when
> >> using a large Php framework thousands of declarations each one in it's
> >> own top context gets loaded - and that takes here 15sec+.
> >> So my idea is to load the declaration only when it's visible, and use
> >> the data from CodeModel for the others. But the problem is sorting
> >> that currently used the same role & column as for display is used.
> >
> > But it's only that slow when completing for the first time, no?
> 
> Yes. But that doesn't help, it takes ages - it needs to be improved.
But at some point you will need those declarations anyway. You could also 
simply preload all those declarations in the background once you open the 
first file.

> > Yes the kate
> > completion model uses the same colums for sorting + display, and it only
> > retrieves the text once without asking again later on, so there is not
> > much lazy loading that can be done right now.
> 
> we can change it :D
> But still the lazy loading is much more context, the whole filtering
> has to be implemented differently.
That would be a lot of work and I'm not sure it would be worth it, given that 
there is simpler alternatives.

Greetings, David