Only by chance I discovered that this option is visible but only when inside home. Before that I always edited dolphinrc to reenable it.
I think it's less confusing to always show it but toggle its enabled state
Details
Details
- Reviewers
emmanuelp ngraham - Group Reviewers
Dolphin - Commits
- R318:3cb3d58fbf0a: folderspanel context-menu option "Limit to Home Directory" should be always…
compile and run
show folderspanel context-menu in different places
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Branch
- arcpatch-D9662_1
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
Doesn't work 100% for me: After I uncheck "Limit to home directory", the menu item becomes permanently disabled and I can't re-enable it.
Comment Actions
Even when you right-click home or a folder inside of it?
I re-checked and it still works as expected here.
| src/panels/folders/treeviewcontextmenu.cpp | ||
|---|---|---|
| 130–136 | I have also tested const bool enableLimitToHomeDirectory = url.isLocalFile(); It looks even more consistent to me. With one exception:
folderpanel stays as is when checking 'Limit to ...'. | |
Comment Actions
I see, the menu item iss only active when you right-click on a path in your home directory. I would support making it always active and jumping you back to your home directory if you're outside it.
Comment Actions
Does a protocol (except 'file://') exist that relies on/utilizes the 'Show hidden folders'-option?
I've checked some but didn't find any.
If none exists, 'Show hidden folders'-option should behave same way as 'Limit to...'.
This comment was removed by michaelh.
Comment Actions
Cannot find the dolphin API Documentation, specially FoldersPanel class. Where is it?
Comment Actions
- Introduced optional parameter to FoldersPanel::loadTree()
- Added navigate home functionality
- Reordered if.. block
- Condition to enable 'Limit to...' option
Comment Actions
Much better. Now the menu item is always available, and activating it jumps you to the home directory if you're outside of it. And you even checked for the case where's you're inside a subdirectory of ~.
Comment Actions
I'm late to the party, but oh well.
| src/panels/folders/folderspanel.cpp | ||
|---|---|---|
| 330 ↗ | (On Diff #25327) | While at it, use QStringLiteral() |
| 352–354 ↗ | (On Diff #25327) | if (jump) is enough, no need to compare with true. |
| src/panels/folders/folderspanel.h | ||
| 93 ↗ | (On Diff #25327) | Bools in APIs are bad for readability, better use enums (e.g. loadUrl(url, FoldersPanel::AllowJumpHome)). |
Comment Actions
@michaelh, would you care to prepare another patch to address @elvisangelaccio's comments?
| CMakeLists.txt | ||
|---|---|---|
| 6 | Don't touch this; the release process manages it. | |
Comment Actions
The new diff is prepared. To be safe I did
$ git pull $ git checkout -b folderpanel-refac origin/Applications/17.12
(made changes)
git diff looks ok
As this one is already closed. I am reluctant to
arc diff
| CMakeLists.txt | ||
|---|---|---|
| 6 | After the splendid time we had yesterday sorting out the diff-mess I produced, I can honestly say "I did not do that on purpose. And, Your Honour, I hereby swear I will never change cmake files (except for a good reason)" :) | |
| src/panels/folders/folderspanel.cpp | ||
| 330 ↗ | (On Diff #25327) | Should this have the same effect as baseUrl = QUrl::fromLocalFile(QDir::rootPath()); I would like to refactor this if..block a little bit. |
| src/panels/folders/folderspanel.h | ||
| 93 ↗ | (On Diff #25327) | Please explain this a little bit. Because by looking at this code I can see no advantage. |