elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
placesitem.cpp:159
+ const QString urlScheme = url().scheme();
+ return (urlScheme.contains(QStringLiteral("search")) || urlScheme.contains(QStringLiteral("timeline")));
}
`contains()` has a `QLatin1String` overload which should be preferred.
placesitemmodel.cpp:54-57
-#ifdef HAVE_BALOO
- #include <Baloo/Query>
- #include <Baloo/IndexerConfig>
-#endif
Please also remove `#include <config-baloo.h>` from the header file
placesitemmodel.cpp:85-86
{
- qDeleteAll(m_bookmarkedItems);
- m_bookmarkedItems.clear();
+ delete m_sourceModel;
+ m_sourceModel = nullptr;
}
Maybe we can use a QScopedPointer for `m_sourceModel`?
placesitemmodel.cpp:148
- for (int i = 0; i < count(); ++i) {
- const QUrl itemUrl = placesItem(i)->url();
- if (url == itemUrl) {
- // We can't find a closer one, so stop here.
- foundIndex = i;
+// look for the coorrect position for the item based on source model
+void PlacesItemModel::insertSortedItem(PlacesItem* item)
typo: correct
placesitemmodel.cpp:198
+{
+ const QModelIndex sIndex = mapToSource(index);
+ const PlacesItem *changedItem = placesItem(mapFromSource(sIndex));
Please call it `sourceIndex`
placesitemmodel.cpp:439
+ // Create default view-properties for all "Search For" and "Recently Saved" bookmarks
+ // in case if the user has not already created custom view-properties for a corresponding
+ // query yet.
Sounds like `if` should not be in this sentence.
placesitemmodel.cpp:551
-void PlacesItemModel::loadBookmarks()
-{
- KBookmarkGroup root = m_bookmarkManager->root();
- KBookmark bookmark = root.first();
- QSet<QString> devices = m_availableDevices;
+ const int blockSize = (end - start) + 1;
remove one space before `+`
placesitemmodel.cpp:571
+ for (int r = topLeft.row(); r <= bottomRight.row(); r++) {
+ const QModelIndex sIndex = m_sourceModel->index(r, 0);
+ const KBookmark bookmark = m_sourceModel->bookmarkForIndex(sIndex);
`sourceIndex`
placesitemmodel.cpp:672
int dropIndex = index;
- const PlacesItem::GroupType type = item->groupType();
+ const QString type = item->group();
Please call it `group`
placesitemmodel.cpp:853-855
- // As long as the KFilePlacesView from kdelibs is available, the system-bookmarks
- // for "Recently Saved" and "Search For" should be a setting available only
- // in the Places Panel (see description of AppNamePrefix for more details).
There is a similar comment where we define `AppNamePrefix`, should that be also removed?
placesitemmodel.h:68-69
+ /**
+ */
typo: hidden
placesitemmodel.h:122
- void clear() override;
+ virtual void clear() Q_DECL_OVERRIDE;
`override`
placesitemmodeltest.cpp:152
QStringList urls;
- for (int row = 0; row < m_model->count(); ++row) {
- urls << m_model->placesItem(row)->url().toDisplayString(QUrl::PreferLocalFile);
+ if (model == nullptr) {
+ model = m_model;
`!model` ?
placesitemmodeltest.cpp:193
+{
+ // user hardcoded path to avoid removal of any user persnonal data
+ QDir dir(QStringLiteral("/home/renato/.qttest/share/placesitemmodeltest"));
typo: personal
REPOSITORY
R318 Dolphin
REVISION DETAIL
https://phabricator.kde.org/D8855
To: renatoo, elvisangelaccio, emmanuelp, mlaurent
Cc: mlaurent, anthonyfieroni, nicolasfella, ngraham, #dolphin