D17270: [KUrlNavigator] List subdirs of a parent folder of an archive

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kurlnavigator.h:297
> +     */
> +    bool isInsideCompressedPath(const QUrl &path) const;
> +

I'd make this function `static`, since it doesn't depend on the status of a specific `KUrlNavigator` instance.

To do so, `isCompressedPath()` needs to become a (static) free function.

> kurlnavigatorbutton.cpp:416
> +        auto kurlnavigator = qobject_cast<KUrlNavigator*>(parent());
> +        if (kurlnavigator->isInsideCompressedPath(kurlnavigator->locationUrl())) {
> +            // We are in an archive, check whether the subdir we have to list is part of the archive

What if the parent is not a `KUrlNavigator`?

> thsurrel wrote in kurlnavigatorbutton.cpp:414
> What would be the clean way to expose this function ? I wouldn't like to duplicate the code.
> And btw, the "if ((m_url.scheme() == QLatin1String("tar")) || (m_url.scheme() == QLatin1String("zip")))" condition was taken from KUrlNavigator::setLocationUrl, so there must be a bugfix to be made there as well with the 'krarc' case gregormi reports.

> What would be the clean way to expose this function ? I wouldn't like to duplicate the code.

Sorry for the late reply. One way could be a public static function, either in `KUrlNavigator` or `KUrlNavigatorButton`.

REPOSITORY
  R241 KIO

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

To: thsurrel, #frameworks
Cc: elvisangelaccio, gregormi, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181216/dc9db4b5/attachment.html>