[Differential] [Requested Changes To] D3170: Centralize code that deals with ideal tool view actions in one class

kfunk (Kevin Funk) noreply at phabricator.kde.org
Wed Oct 26 17:52:02 UTC 2016


kfunk requested changes to this revision.
kfunk added a reviewer: kfunk.
kfunk added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> idealbuttonbarwidget.cpp:44
> +public:
> +    ToolViewAction(IdealDockWidget *dock, QObject* parent) : QAction(parent), m_dock(dock) {
> +        setCheckable(true);

Nitpick: `{` in next line

> idealbuttonbarwidget.cpp:61
> +
> +    IdealDockWidget *dockWidget() const {
> +        Q_ASSERT(m_dock);

Nitpick: `{` in next line

> idealbuttonbarwidget.cpp:76
> +
> +    void refreshText() {
> +        const auto widget = m_dock->view()->widget();

Nitpick: `{` in next line

> idealbuttonbarwidget.cpp:82
> +        else
> +            setText(title+" *");
> +    }

Hmm, not sure I like the asterisk here. To me, an asterisk usually means: "This item was modified".

Can we find something better? Maybe use the disabled-mode of the icon (cf. `QIcon::Mode`)?

> idealbuttonbarwidget.cpp:195
>  
> -void IdealButtonBarWidget::showWidget(QAction *widgetAction, bool checked)
> +void IdealButtonBarWidget::showWidget(QAction *_widgetAction, bool checked)
>  {

`_widgetAction` -> `widgetAction`

> idealbuttonbarwidget.cpp:197
>  {
> -    IdealDockWidget *widget = _widgets.value(widgetAction);
> -    Q_ASSERT(widget);
> +    ToolViewAction* widgetAction = dynamic_cast<ToolViewAction*>(_widgetAction);
>  

`auto toolViewAction = ...`

> idealbuttonbarwidget.cpp:226
>  {
> -    QAction *action = qobject_cast<QAction *>(event->action());
> -    if (! action)
> +    ToolViewAction *action = dynamic_cast<ToolViewAction *>(event->action());
> +    if (!action)

Dito

> idealbuttonbarwidget.cpp:281
>  {
> -    return _widgets.value(action);
> +    ToolViewAction *action = dynamic_cast<ToolViewAction *>(_action);
> +    return action->dockWidget();

Dito

REPOSITORY
  rKDEVPLATFORM KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D3170

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: apol, brauch, #kdevelop, kfunk
Cc: kfunk, antonanikin, brauch, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161026/ff65eba7/attachment-0001.html>


More information about the KDevelop-devel mailing list