Added an option on PlacesPanel context menu to show or hide the entire
group of places.
Depends on D8855
Added an option on PlacesPanel context menu to show or hide the entire
group of places.
Depends on D8855
Open Donlphin and use PlacesPanel context menu to hide and show groups
Lint Skipped |
Unit Tests Skipped |
src/panels/places/placesitemmodel.h | ||
---|---|---|
24 | #include <KFilePlacesModel> ? |
I agree with the suggestion by Elvis, otherwise lgtm besides one more suggestion for potential cleanup
src/panels/places/placesitemmodel.cpp | ||
---|---|---|
477 | usually it would be preferred if you could add a proper role for the group type. Then you could just do auto groupType = index.data(KFilePlacesModel::GroupTypeRole).value<KFilePlacesModel::GroupType>(); item->setGroupHidden(model->isGrouHidden(groupType)); at this point, it's then even better to just make this possible: auto groupHidden = index.data(KFilePlacesModel::GroupHiddenRole).toBool(); item->setGroupHidden(groupHidden); |
Used new KFilePlacesModel::GroupHiddenRole to retrieve information instead of call the model method
I do not think this is the problem since that after run: arc patch D9242. I got both changes on my tree.
I get:
$ arc patch D9242 INFO Base commit is not in local repository; trying to fetch. Created and checked out branch arcpatch-D9242. INFO Base commit is not in local repository; trying to fetch. Checking patch src/filewidgets/kfileplacesmodel.h... error: src/filewidgets/kfileplacesmodel.h: does not exist in index Checking patch src/filewidgets/kfileplacesmodel.cpp... error: src/filewidgets/kfileplacesmodel.cpp: does not exist in index Checking patch autotests/kfileplacesmodeltest.cpp... error: autotests/kfileplacesmodeltest.cpp: does not exist in index Patch Failed! Usage Exception: Unable to apply patch!
The summary has a dependency on D9252, which makes arc patch fetch it. However, the dependency itself is based on a git commit hash from R241 KIO and does not relate to the current git repo, so things go wrong. Try removing the dependency from the summary and then test with a clean repo checkout.
Alternatively, try arc patch --skip-dependencies.
@elvisangelaccio I removed the KIO dependency from the summary could you try again? Or use the solution proposed by @rkflx
And if I manually download the patch, it fails with a similar error when applying it.
If you use --skip-dependencies, you'd have to apply D8855 manually too, of course. Then it works for me.
The reason arc patch D9242 still does not work is because the dependency on D9252 is still there. Just removing it from the summary is not enough apparently, you have to use "Edit Related Revisions". It is all very logical once you stumbled upon this the first time…
Try this, I hope then it will work. Also, look at the "Stack" tab to see what's going on…
Patch seems to work fine, nice job!
src/panels/places/placespanel.cpp | ||
---|---|---|
340 | Should we add this new action/option in dolphin_placespanelsettings.kcfg ? |
(OT)
Interesting that plain arc patch still fails. --trace shows it tries to fetch 7ef5e26f2f474f3245dedfbb4aa2e5c1acceb951 from R241 KIO, which is the parent commit of D8243, i.e. a transitive dependency of D8855. The error message matches exactly the files from D8243, too.
What I cannot figure out is why applying D8855 works, and for D9242 it fails. Sounds like a bug in arc, of which there are plenty in upstream Phab. In general cross-repo dependencies are not a sensible thing to have, so perhaps cleaning this up (i.e. fixing the dependencies of D8855) would help. However, I will stop here as this is not worth the effort ;)
src/panels/places/placespanel.cpp | ||
---|---|---|
340 | I do not think that is necessary we already store the visible/hide group on the bookmark file, storing it in another file will require a new level of abstraction to filter it and make it even more complex. |