D20026: Go up in folder hierachy when in "edit mode"

[Nathaniel Graham]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190326/eaef7855/attachment-0001.html>