Review Request: Proper auto-completion in switch statements

Olivier Jean de Gaalon olivier.jg at gmail.com
Sun Jul 8 02:29:53 UTC 2012



> On July 7, 2012, 2:07 a.m., Olivier Jean de Gaalon wrote:
> > Alright, brilliant work. Not having m_expressionResult available for case access until the last minute is unfortunate, but at least it's commented and I don't see any prettier way of doing this without some serous reworking.
> > prependScopePrefix seems like a good feature to me.
> > Tests are extensive and all in order.
> > 
> > Only one last thing, just remove the requiredPrefix function. For the conditional in specialItemsForArgumentType just use "m_duContext->findDeclarations(enumDecl->identifier()).contains(enumDecl)".
> > 
> > With that done, and whitespace trimmed, Ship It.
> 
> Ivan Shapovalov wrote:
>     I'd rather use CodeCompletionContext::visibleFromWithin() to do that conditional... Is it OK?
>     And - it does not apply after rtrim'ing. I've manually trimmed all places where Git complained about whitespace errors.
>     
>     Anyway, uploading a formatted patch in hope that these issues will be OK. Thanks for the review!

Could you remove the requiredPrefix function? It's dead code now.


- Olivier Jean de


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105410/#review15485
-----------------------------------------------------------


On July 7, 2012, 7:04 a.m., Ivan Shapovalov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105410/
> -----------------------------------------------------------
> 
> (Updated July 7, 2012, 7:04 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Improve code completion within switch statements.
> 
> 1) Resolve type of the switch'd expression correctly
>  - in switchExpressionType(), replaced evaluateType() with evaluateExpression()
> 
> 2) Add completion items for enumerations declared in different scopes
>  - this required adding another condition branch in the end of CodeCompletionContext::standardAccessCompletionItems(),
> which in turn required proper setting of m_expressionResult in CaseAccess contexts,
> which required moving switchExpressionType() to CodeCompletionContext::caseAccessCompletionItems() and re-ordering some code.
> 
> 
> Diffs
> -----
> 
>   languages/cpp/codecompletion/context.cpp 33dcad1 
>   languages/cpp/codecompletion/item.h 4735f2a 
>   languages/cpp/codecompletion/item.cpp affd4e6 
>   languages/cpp/tests/test_cppcodecompletion.h 20a70cb 
>   languages/cpp/tests/test_cppcodecompletion.cpp ec82d2d 
> 
> Diff: http://git.reviewboard.kde.org/r/105410/diff/
> 
> 
> Testing
> -------
> 
> There are unit-tests, but they do not work since DUChainBase::createRangeMoving(), which is used to parse switch'd expression type, does not work without an editor.
> 
> 
> Thanks,
> 
> Ivan Shapovalov
> 
>

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


More information about the KDevelop-devel mailing list