@David Nolden Question re: bug 244774

[Olivier JG]

On 12/31/2010 02:30 AM, David Nolden wrote:
> 2010/12/18 Olivier JG<olivier.jg at gmail.com>:
>> Looking at this bug: https://bugs.kde.org/show_bug.cgi?id=244774
>>
>> The problem is at context.cpp:369
>>
>> if(!expressionPrefix.isEmpty()&&  (expressionPrefix.endsWith('>') ||...
>>
>> which catches the greater than operator in this line (for example):
>>
>> bool true = obj1>  this
>>
>> causing the valid expression "this" to be replaced with junk from the
>> prefix.
>>
>> I'd fix it, but I'm not totally sure what that if statement is doing. Given
>> context.cpp:369:
>>
>> if(!expressionPrefix.isEmpty()&&
>>     (expressionPrefix.endsWith('>') ||<<<<<<<<<  templates?
>>     expressionPrefix[expressionPrefix.length()-1].isLetterOrNumber() ||
>>     expressionPrefix[expressionPrefix.length()-1] == '_'))<<<<<<<<  ???
>> What do the two marked expressions doing? I imagine the first is to catch
>> templates (not certain though), but I'm stumped on the second.
>> Removing them doesn't make any tests fail (I intend to remedy that), so I'm
>> asking you :).
>>
>> -Olivier JG
> Look at the comment above. This code is about giving constructor
> completion in case of "ClassType classInstance(". I guess in this
> case, expressionPrefix is "ClassType". This, the code tries to
> identify whether that supposed "ClassType" prefix really is a type
> identifier, or whether it is some other expression. For example, the
> prefix could be "anotherFunctionCall(", "someValue +", or whatever, in
> which case the if-statement above will fail.
>
> Thus, the statement simply tests whether the last character of the
> prefix is a valid last character of a type identifier (thus the
> ltterOrNumber() and "_"), and the ">" is required in case it is a
> template type.
>
> I guess this check is a bit too stupid.
>
> Strange that no test fails when romoving them, this shows that we need
> some more tests. Given types with constructors named "Type1, Type_,
> template<class T>  Type":
> CompletionTestHelper("Type1 instance(") ->  make sure that we have the
> constructor of Type1 in the completion list
> CompletionTestHelper("Type_ instance(") ->  make sure that we have the
> constructor of Type_ in the completion list
> CompletionTestHelper("Type<Type1>  instance(") ->  make sure that we
> have the constructor of Type<Type1>  in the completion list
>
> Greetings, David
Yeah, I figured out what it was doing, sorry to bother. I imagine this 
will just require an ExpressionParser to verify the type?

I'm actually trying to refactor the cpp code completion context a bit 
now, so I'll try to fix this then (as well as a couple other things). 
I'll also be adding more tests of course.

-Olivier JG