Selects the first ancestor from the current directory whenever a tree is created using Create New | Folder.
Let me know in case I missed any tests for this otherwise I'll see to it that I try add some.
Details
Details
- Reviewers
ngraham elvisangelaccio rkflx
- Navigate to a directory
- Right click, create new -> folder. Use a name as such "a/b/c"
- Observe that the tree is created, directory a appears in the current directory
Expected: Dolphin selects directory "a"
Before patch: Selection is empty.
Actual: "a" is selected.
Create dir that already exists (~/Desktop)
Create subitem in a dir that exists (~/x exists -> create ~/x/y/z)
Create dir that doesn't exist (~/nothere)
Create path that doesn't exist (~/does/not/exist/yet)
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Branch
- arcpatch-D11304
- Lint
No Linters Available - Unit
No Unit Test Coverage
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1340 ↗ | (On Diff #29447) | Are we sure m_model->directory() does not already end with a slash? |
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1340 ↗ | (On Diff #29447) | From the looks of it the trailing path delimiter is stripped at KIO's KCoreDirLister, which is what KFileItemModel uses to get the url. https://cgit.kde.org/kio.git/tree/src/core/kcoredirlister.cpp#n118 |
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1340 ↗ | (On Diff #29447) | Ok. Then please let's do it like this: const QString dir = QStringLiteral("%1/").arg(m_model->directory().toString()); Also, which dir? Maybe if we call it modelDir it's a bit clearer. |
| 1343 ↗ | (On Diff #29447) | name of what? Maybe we can call it ancestorName ? Also, don't use QDir::separator(), I think it might not work on Windows. From the its doc: You do not need to use this function to build file paths. If you always use "/", Qt will translate your paths to conform to the underlying operating system. If you want to display paths to the user using their operating system's separator use toNativeSeparators(). |
Comment Actions
Moved the code which finds the child to a separate function + added a test suite for it.
- add commonancestor function + tests
- rename to directoryfindchilditem
- rename test
- fix ident
- refactor
- update moc file
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1816 | @elvisangelaccio Without this an item which already existed would not be selected(Say directory contains item a, and a/b/c is created). Not 100% sure if this is the right way to tackle this. | |
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1816 | No, it's not. Sadly after this change the dolphin view no longer scrolls the a newly created folder (it does select it, though). | |
Comment Actions
- Merge remote-tracking branch 'origin/master' into arcpatch-D11304
- Fix item would not scroll and selection issue
Comment Actions
Oops, sorry I missed this!
In the case that the user creates a/b/c, I think what would actually be most expected is to change the view to show a/b/, and then select c.