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