Review Request: Plasma::AbstractRunner::Syntax

[Kevin Ottens]

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



trunk/KDE/kdelibs/plasma/abstractrunner.h
<http://reviewboard.kde.org/r/372/#comment368>

    Why not a default ctor instead? You've accessors anyway and it's mainly a "data store" class.



trunk/KDE/kdelibs/plasma/abstractrunner.h
<http://reviewboard.kde.org/r/372/#comment369>

    I'd go for a setExampleQueries(const QStringList &queries) instead to make it symmetrical. Otherwise one would wonder how to reset the query list, etc. You'd end up having the most common QList services in the Syntax API.



trunk/KDE/kdelibs/plasma/abstractrunner.h
<http://reviewboard.kde.org/r/372/#comment370>

    Probably requires a plural?



trunk/KDE/kdelibs/plasma/abstractrunner.h
<http://reviewboard.kde.org/r/372/#comment371>

    Again I'd go for a setter for QList<Syntax> instead of (add|clear)Syntax() methods.


- Kevin


On 2009-03-20 13:13:37, Aaron Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/372/
> -----------------------------------------------------------
> 
> (Updated 2009-03-20 13:13:37)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Allows runners to register their syntax so that it can be displayed to the user, and perhaps even used in the future to automate the process of deciding whether a runner should actually be activated or not.
> 
> :q: is used to mean "the search term" in the texts.
> 
> A class for Syntax was added rather than a more simple addSyntax("example query", "description") API in AbstractRunner so that we can extend what is associated with Syntax in future versions if we wish. It also opens the door for runners to subclass Syntax and have the syntax objects update dynamically based on settings or environment.
> 
> apidox are missing.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/plasma/abstractrunner.h 938702 
>   trunk/KDE/kdelibs/plasma/abstractrunner.cpp 938702 
>   trunk/KDE/kdelibs/plasma/runnermanager.cpp 938756 
> 
> Diff: http://reviewboard.kde.org/r/372/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron
> 
>