Review Request 119407: Use the DUChain to offer better navigation widgets

[Denis Steckelmacher]

> On July 22, 2014, 8:04 p.m., Sven Brauch wrote:
> > > How can I lock the foreground lock?
> > 
> > There's a ForegroundLock class in KDevPlatform.
> > 
> > 
> > > Is KDevQmlJSPlugin::specialLanguageObjectNavigationWidget called in the foreground thread?
> > 
> > Not sure, just compare QObject::thread() to QApplication::instance()->thread() -- that should tell you.
> > 
> > 
> > Otherwise, I think this is a very nice feature. However, as we discussed a while ago (not sure if you agreed though?) I still think some of the widgets should be removed because they are probably more distracting than useful. Margin for example just isn't that great unless you manage to render it with an actual preview of the scene.

I've put an assert on "QThread::currentThread() == QCoreApplication::instance()->thread()", and the assertion succeeded. So, I think that KDevQmlJSPlugin::specialLanguageObjectNavigationWidget runs in the foreground thread and that no locking is needed. Do you want me to use ForegroundLock so that we are completely sure that everything is correct?

Regarding the widgets, I totally agree that some of them may not be that useful. In fact, I've already removed two or three widgets (but I don't remember which ones). "Margins" is the less useful one at this time, but I'd still keep it, because margins are difficult to imagine. Maybe the widget should be modified, though, because I don't find the 3x3 grid very relevant.  I have some ideas that I will test once this patch is merged.


- Denis


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


On July 22, 2014, 4:33 p.m., Denis Steckelmacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119407/
> -----------------------------------------------------------
> 
> (Updated July 22, 2014, 4:33 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-qmljs
> 
> 
> Description
> -------
> 
> The QML/JS plugin can show navigation widgets that can be used to edit specific QML properties like width/height, spacing, color, etc. This patch extends these widgets so that they know the declaration (and hence the type) of the property being modified.
> 
> This allows two things: filters can be more specific (for instance, the font-family helper widget is displayed for the "family" property of "font", and not for anything else), and filters can be created for certain property types, independently of the property names (for instance, the color picker is now shown for any property that has the type color).
> 
> This patch uses DUChainUtils::itemUnderCursor to get the declaration used at a given position in a source file. The documentation of this function says that the function can only be called from the foreground thread, or with the foreground lock held. How can I lock the foreground lock? Is KDevQmlJSPlugin::specialLanguageObjectNavigationWidget called in the foreground thread?
> 
> 
> Diffs
> -----
> 
>   kdevqmljsplugin.cpp a917bf2 
>   navigation/propertypreviewwidget.h ca416a9 
>   navigation/propertypreviewwidget.cpp 6d39e7b 
> 
> Diff: https://git.reviewboard.kde.org/r/119407/diff/
> 
> 
> Testing
> -------
> 
> Manual testing has shown that everything works as expected. Here is my test file, with the results in comments:
> 
>     Item {
>         id: root
>         width: 3              // Shows the "Width" widget
> 
>         property color clr    // This line and the following ones don't show anything (this patch fixes a small bug: SimpleRange() does not create an invalid range, but a range from (0, 0) to (0, 0)
>         property float margins
>         property font font
>         property string family
> 
>         clr: "#78d0aa"        // Shows the color picker
> 
>         anchors.margins: 13   // Shows the "Margins" widget
>         anchors {
>             margins: 13       // Shows the "Margins" widget
>             leftMargin: 16    // Shows the "Margins" widget
>         }
> 
>         margins: 34           // Shows the standard navigation widget (no property is recognized because there is a filter for QQuickAnchors.margins)
>         opacity: 0.5          // Shows the "Opacity" widget
>         font.family: "Arial"  // Shows the "FontFamily" widget
> 
> 
>         family: "Me"          // Shows the standard navigation widget
>     }
> 
> This patch is already quite useful, but will be even more when the QML/JS plugin will be based on Qt5 and will be able to use QtQuick.Controls. When this will be the case, more advanced helper widgets will be possible (URL, combo boxes for font families and enumeration values, etc). Having type-dependend widgets will allow some nice things like a widget that allows the user to preview alignments (vertical/horizontal left/right/top/bottom/center) regardless of the name of the property that will contain the alignment.
> 
> 
> Thanks,
> 
> Denis Steckelmacher
> 
>

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