D11875: Revive the KLook patch for dolphin

Vlad Zagorodniy noreply at phabricator.kde.org
Thu May 24 18:47:43 BST 2018


zzag added inline comments.

INLINE COMMENTS

> dolphinmainwindow.cpp:1659
> +    int index = 0;
> +    if ( m_activeViewContainer->view()->selectedItemsCount() > 1) {
> +        list = m_activeViewContainer->view()->selectedItems();

Coding style nitpick: no whitespace after `(`.

> dolphinmainwindow.cpp:1666
> +    QStringList urlList;
> +    foreach(KFileItem item, list)
> +    {

Maybe range based for loop?

> dolphinmainwindow.cpp:1671
> +    urlList.append("-i " + QString::number(index , 10));
> +    if (!list.isEmpty())
> +    {

Coding style nitpick: `if (!list.isEmpty()) {`. Same down below.

> dolphinmainwindow.cpp:1700
> +    }
> +#endif
> +}

#else
  Q_UNUSED(old)
  Q_UNUSED(now)
  #endif

> kfileitemlistwidget.cpp:26
>  #include <KLocalizedString>
> +#include <KIconLoader>
>  

Maybe, `KIconLoader` should be between `KFormat` and `KLocalizedString`? (to keep order)

> kfileitemlistwidget.h:48
>      static KItemListWidgetInformant* createInformant();
> +    virtual QRectF klookToggleRect() const override;
>  

`virtual` is redundant.

> kitemlistcontroller.cpp:573
>      }
> +    if (m_view->isAboveKlookToggle(m_pressedIndex, m_pressedMousePos)){
> +        m_selectionManager->setSelected(m_pressedIndex, 1, KItemListSelectionManager::Select);

Coding style nitpick: missing whitespace between `)` and `{`

> kitemlistklooktoggle.cpp:51
> +}
> +*/
> +void KItemListKlookToggle::paint(QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget)

Delete commented code? (or add a comment describing why it shouldn't be delted)

> kitemlistwidget.h:35
>  class KItemListView;
> +class KItemListKlookToggle;
>  class QPropertyAnimation;

IMHO, it would be great to have the forward declared classes in order.

REPOSITORY
  R318 Dolphin

BRANCH
  klook

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

To: fabiank, cfeck
Cc: zzag, cfeck, kfm-devel, shevchuk, ngraham, spoorun, navarromorales, isidorov, firef, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180524/fae3dd77/attachment.htm>


More information about the kfm-devel mailing list