ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.
Thanks, this works great in Dolphin. However it does not work properly in
Gwenview; the URL Navigator switches back to breadcrumbs mode every time the Up
arrow is pressed. I've also got come coding and style suggestions:
INLINE COMMENTS
> CMakeLists.txt:15
> defaultviewadapter.cpp
> -
> kdiroperator.cpp
Unrelated whitespace change
> CMakeLists.txt:42
> kurlnavigatorpathselectoreventfilter.cpp
> -
> -
Unrelated whitespace change
> kurlnavigator.cpp:54
> #include <qmimedatabase.h>
> +#include <QDebug>
> #include <QMimeData>
Don't want this in production code
> kurlnavigator.cpp:124
> +
>
> /**
Unrelated whitespace change
> kurlnavigator.cpp:348
> + */
> + slotToggleEditableButtonPressed();
> + QTimer::singleShot(10, q->editor()->lineEdit(), SLOT(setFocus()));
This works in Dolphin, but causes the navigator to switch back to breadcrumbs
mode in Gwenview. I think we need to instead fix whatever bug is causing the
desire for thus ugly hack. :)
> kurlnavigator.cpp:387
> + if (slash == -1)
> + return QString();
> + else if (slash == 0)
Coding style: don't omit braces for single-line conditionals
> kurlnavigator.cpp:402
> + hasParent = (currentDirectory !=
> QUrl(parentDirectory(currentDirectory.path())));
> + qInfo() << currentDirectory.path();
> + currentDirectory = QUrl(parentDirectory(currentDirectory.path()));
Don't want this in production code
> kurlnavigator.cpp:425
> }
> -
> switchView();
Unrelated whitespace change
> kurlnavigator.cpp:474
> + } else {
> + m_pathBox->installEventFilter(q);
> }
This feels like a hack
> kurlnavigator.cpp:561
> q->setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Fixed);
> -
> m_pathBox->show();
Unrelated whitespace change
> kurlnavigator.cpp:1287
> + emit keyUpPressed();
> + return true;
> + } else if (actualEvent->key() == Qt::Key_Down) {
You could remove the `true` from these
> kurlnavigator.h:495
> Private *const d;
> -
> Q_DISABLE_COPY(KUrlNavigator)
Unrelated whitespace change
> kurlcombobox.h:113
> void setUrls(const QStringList &urls);
> + void setUrls(const QStringList &urls, bool removeOld);
>
Too much indentation
> kurlcombobox.h:123
> void setUrls(const QStringList &urls, OverLoadResolving remove);
> + void setUrls(const QStringList &urls, OverLoadResolving remove, bool
> removeOld);
>
In general we prefer using Enums rather than bools for function arguments. It
makes the code much more readable.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D20026
To: krutovmikhail, ngraham, #dolphin
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
