Review Request 119208: Use DUContext::Class instead of DUContext::Namespace when declaring namespace contexts

[Milian Wolff]

> On July 10, 2014, 10:18 a.m., Milian Wolff wrote:
> > This will mean you'll have to special-case these imports in the navigation windows though, no? There, you do want to say that it is a namespace (and not a class/type), or?
> > 
> > Furthermore, unrelated to this review request, could you explain me again why you create alias declarations instead of importing the context of the imported file into the namespace/class context?
> 
> Denis Steckelmacher wrote:
>     Even if the type of the context is set to Class, the kind of the declaration remains Namespace. The code-completion items and the navigation widget therefore still show the proper icon and "namespace ...". Is there a place that may use the type of the context and that I should check?
>     
>     Regarding aliased declarations, I first tried to import the context of the file in the namespace, but then I discovered that I needed some sort of filtering. For instance, when importing a file, the namespace alias declarations of this file should not be imported (typing "MyFile." should offer completion items for every local declaration of MyFile, but not for "MyFile.QtQuick", "MyFile.MyOtherFile", etc).

Regarding navigation: ok, cool. if it works that sounds OK.

Regarding alias declarations: we should rethink that and probably filter it in the code completion then, instead of spamming the duchain with alias declarations.


- Milian


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


On July 10, 2014, 10:04 a.m., Denis Steckelmacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119208/
> -----------------------------------------------------------
> 
> (Updated July 10, 2014, 10:04 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-qmljs
> 
> 
> Description
> -------
> 
> KDevPlatform sometimes special-cases contexts having the Namespace type. This was not a problem until I discovered that this QML snippet does not work:
> 
>     :::javascript
>     import "myfile.js" as MyFile
> 
>     Item {
>         onTest: console.log(MyFile.myVariable)
>     }
> 
> In order to better see which contexts are used, here is the same snippet in a C++-like language:
> 
>     :::cpp
>     namespace MyFile {
>         alias myVariable = "myVariable in myfile.js";
>     }
>     
>     class __ : public Item {
>         void onTest() {
>             console.log(MyFile::myVariable);
>         }
>     };
> 
> The line starting with "onTest" calls findDeclarations(myVariable) on MyFile, that is a namespace (and has a namespace context). myVariable is a local declaration of MyFile, but is not returned by findDeclarations because an if statement skips findLocalDeclarations for Namespace contexts. A comment in language/duchain/ducontext.cpp says that declarations in namespace contexts are resolved in the top-context, but this cannot happen in my case because I set the DontSearchInParent flag when looking for object members.
> 
> This patch work-arounds this issue by giving a Class type to the internal contexts of namespace declarations. This way, KDevPlatform does not special-case the contexts and works as expected.
> 
> 
> Diffs
> -----
> 
>   duchain/declarationbuilder.cpp 8927633 
> 
> Diff: https://git.reviewboard.kde.org/r/119208/diff/
> 
> 
> Testing
> -------
> 
> The whole testsuite passes. The feature shown above (importing Javascript files) is not added by this patch, but is made to work with this patch applied.
> 
> 
> Thanks,
> 
> Denis Steckelmacher
> 
>

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