D19926: Add Bookmark Handling

Elvis Angelaccio noreply at phabricator.kde.org
Thu Apr 25 12:30:52 BST 2019


elvisangelaccio added inline comments.

INLINE COMMENTS

> dolphinbookmarkhandler.cpp:42
> +        if (genericDataLocation.isEmpty()) {
> +            qWarning() << "GenericDataLocation is empty! Bookmarks might now be saved correctly.";
> +        }

typo: "might not"

But actually I'd just say "will not".

> dolphinbookmarkhandler.cpp:84
> +{
> +    QList<DolphinViewContainer*> viewContainers = m_mainWindow->viewContainers();
> +    QList<FutureBookmark> bookmarks;

Missing `const`, which would also enable us to use the C++11 for loop below.

> dolphinbookmarkhandler.h:51-53
> +    QString title(DolphinViewContainer* viewContainer) const;
> +    QUrl url(DolphinViewContainer* viewContainer) const;
> +    QString icon(DolphinViewContainer* viewContainer) const;

These 3 helper functions could all be `static`.

> dolphinmainwindow.h:78
> +     */
> +    QList<DolphinViewContainer*> viewContainers() const;
> +

We could use `QVector` here, which should be preferred to `QList` for new code.

REPOSITORY
  R318 Dolphin

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

To: hallas, #dolphin, elvisangelaccio, ngraham
Cc: loh.tar, cfeck, hein, kfm-devel, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190425/fb9e504b/attachment.htm>


More information about the kfm-devel mailing list