Differential D17270

[KUrlNavigator] List subdirs of a parent folder of an archive
Needs ReviewPublic
Actions

Authored by thsurrel on Nov 30 2018, 10:23 PM.

Details

Reviewers
None
Group Reviewers
Frameworks
Summary

When we are navigating in an archive, trying to use the navigator
buttons to list the subdir of a parent folder of the archive was
not working because the protocol would still be 'tar' instead of
'file'.

Test Plan

In Dolphin, click on a zip file. Then in the URL bar (breadcrumb
mode), try to list the sub directories of the folder containing
that zip file by clicking on the litlle arrow on the right of the
folder's name.

Diff Detail

Repository
R241 KIO
Branch
arc_urlnavigatorbutton (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6006
Build 6024: arc lint + arc unit
thsurrel created this revision.Nov 30 2018, 10:23 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 30 2018, 10:23 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
thsurrel requested review of this revision.Nov 30 2018, 10:23 PM
thsurrel updated this revision to Diff 46589.Nov 30 2018, 10:24 PM

Fix deleted empty line.

gregormi added a subscriber: gregormi.Nov 30 2018, 10:30 PM
gregormi added inline comments.
src/filewidgets/kurlnavigatorbutton.cpp
414

When I click a zip file I get this in the URL bar:

krarc:/home/gregor/Downloads/kfk_10p.zip/

This could mean that "krarc" should be added to this if statement.

elvisangelaccio added a subscriber: elvisangelaccio.Dec 1 2018, 3:59 PM
elvisangelaccio added inline comments.
src/filewidgets/kurlnavigatorbutton.cpp
414

Or we could use mimetype detection like we do in KUrlNavigator::Private::isCompressedPath().

417

Coding sytle: variable names need to be camelCase.

thsurrel added inline comments.Dec 3 2018, 12:53 PM
src/filewidgets/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.

thsurrel updated this revision to Diff 47545.Dec 13 2018, 9:26 PM

Create KUrlNavigator::isInsideCompressedPath
Use this function instead of comparing the url scheme with tar, zip, ...

elvisangelaccio added inline comments.Dec 16 2018, 11:58 AM
src/filewidgets/kurlnavigator.h
297

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.

src/filewidgets/kurlnavigatorbutton.cpp
414

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.

416

What if the parent is not a KUrlNavigator?

Revision Contents
Changeset List

Diff 47545

View Options

src/filewidgets/kurlnavigator.h

Loading...
View Options

src/filewidgets/kurlnavigator.cpp

Loading...
View Options

src/filewidgets/kurlnavigatorbutton.cpp

Loading...