D25315: KDirModel: implement showing a root node for the requested URL
rrosch added a comment. Tested, no crashes, it works as intended, and it is beautiful! Thank you! REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25315 To: dfaure, stefanocrocco, elvisangelaccio, meven, apol Cc: rrosch, ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D25315: KDirModel: implement showing a root node for the requested URL
rrosch added a comment. Hi, sorry for the lack of responses, for some reason I did not receive any of the notifications except this last one. I will test and comment as soon as I can. Wishing everyone safety and health too. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25315 To: dfaure, stefanocrocco, elvisangelaccio, meven, apol Cc: rrosch, ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D25315: KDirModel: implement showing a root node for the requested URL
rrosch added a comment.
Crash:
kf5.kio.kdirmodel: Items emitted in directory QUrl("file:///home/myuser/")
but that directory isn't in KDirModel! Root directory:
QUrl("file:///home/myuser/")
KCrash: Application 'konqueror' crashing...
This happens right after pressing F9 to bring up the sidebar.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D25315
To: dfaure, stefanocrocco, elvisangelaccio, meven, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D25315: KDirModel: implement showing "/" as a root node, optionally
rrosch added a comment.
> Keep only matters for further calls to openUrl, not the first one. It's
about whether to *add* or *replace* the currently open URL.
> KDirModel takes care of that.
Ah ok, so it's going to do that without me needing to explicitly specify
"Keep"?
>> I couldn't however get the following to work:
>>
>> QModelIndex index = getIndexFromUrl("/home/myuser");
>
> Invalid URL, that's a path, not a URL. You need QUrl::fromLocalFile().
Sorry that was me using shorthand to illustrate the problem, the url in the
code is an absolute url stored in a variable with the schema and all the
necessary things.
>> Which should show the node for "/home" as the root, but is instead giving
me a flat listing of all the child nodes without "/home" as the root.
>
> Oh, hmm, that's not how setRootIndex works (that's a *view* feature, we
can't change that).
> I changed the model to have one more node for "/", while I see now that
what you want is that it *always* shows a root node even when the root is
another directory.
Yeah that was my intent with the above. How come it doesn't work in the view
for `file:///home/myuser` but it works for `file:///`?
In any case, the code here you provided works great for at least `file:///`
and that gets us quite far. How much of a rework would getting it to work with
subdirectories (like `file:///home/myuser`) take?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D25315
To: dfaure, stefanocrocco, elvisangelaccio, meven, apol
Cc: rrosch, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham,
bruns
D27731: Improve KDirModel to avoid showing '+' if there are no subdirs
rrosch accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D27731 To: dfaure, apol, ahmadsamir, meven, rrosch Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D25315: KDirModel: implement showing "/" as a root node, optionally
rrosch added a comment. Ok, I found one of the "implications". With `model->dirLister()->openUrl(m_initURL, KDirLister::Keep);` I can list KIO urls like `remote:`, `font:`, and `applications:`, but with `model->openUrl(m_initURL, KDirModel::ShowRoot);` I cannot, and only seems to display the the local filesystem (specifically it lists "/"). I can probably work around it, but just wanted to pass that info along. I think it would be great if the "root" could be shown for these as well. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25315 To: dfaure, stefanocrocco, elvisangelaccio, meven, apol Cc: rrosch, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D25315: KDirModel: implement showing "/" as a root node, optionally
rrosch added a comment.
Tested on patched KF5 5.64, on Fedora 30 (32bit)... root node shows up! I had
to change the call in my code that was originally:
model->dirLister()->openUrl(QUrl::fromLocalFile("/"), KDirLister::Keep);
to
model->openUrl(QUrl::fromLocalFile("/"), KDirModel::ShowRoot);
and don't really know the implications of the change at this point. (Will it
"Keep"? Does it matter that I now call the model's openURL vs the dirLister's?)
I couldn't however get the following to work:
QModelIndex index = getIndexFromUrl("/home/myuser");
if (index.isValid()) {
treeView->setRootIndex(index.parent());
}
Which should show the node for "/home" as the root, but is instead giving me
a flat listing of all the child nodes without "/home" as the root. Am I doing
something wrong here? That part is called on a slot for the signal
`::expand`. (Also, it would be great to have a setRootPath() that
works in conjunction to expandToUrl or openUrl, to save processing cycles
wasted on loading the parts of the model that won't be shown in the new
rootIndex, like https://doc.qt.io/qt-5/qfilesystemmodel.html#setRootPath )
As it is relevant, I also tested the patch at
http://www.davidfaure.fr/2019/kdirmodel_haschildren.diff for `hasChildren()`,
which works as expected! Thank you!
---
For the curious, I just downloaded the source rpm for `kf5-kio`, installed it
as unprivileged user, edited the .spec file to include the raw diff here as a
the //second// patch, and the raw diff for D25249
as the //first// patch (and copying them
to `~/rpmbuild/SOURCES/` of course). I also had to change the `Release: ` line
to something greater than the one already there so it installs without
conflicts (I changed mine from "1" to "2patched"). Then just
rpmbuild -bb ~/rpmbuild/SPECS/kf5-kio.spec
and install the relevant rpms. No need to rebuild all of KF5 from git (which
I started to attempt and bailed pretty quickly after thinking this might be the
easier/faster/less space-using way. Knowing this process will be useful for
newcomers to help test features on their system without the daunting (and
probably impossible due to limited space) task of compiling all of KF5. This
could be added to the dev documentation somewhere.
In any case, thanks again for implementing the feature! (Next step is to try
to follow the same procedure to test the "crash on close of view" fix you
submitted for Qt5, thanks also for that!)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D25315
To: dfaure, stefanocrocco, elvisangelaccio, meven, apol
Cc: rrosch, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham,
bruns
