If u == "file:///", then u.setPath(u.path() + '/') results in u == "".
Avoid making this transformation for all schemes (thanks @dfaure).
Details
Details
- Reviewers
dfaure - Group Reviewers
Frameworks - Commits
- R241:1cea9463f471: Make it possible to go up to root again
In a File open dialog, go to a nfts disk root, that results in currentUrl = file:///d//
Before, when I pressed the upCd dir (^) I was redirected to my home directory, after I see the root directory.
Diff Detail
Diff Detail
- Repository
- R241 KIO
- Lint
Lint Skipped - Unit
Unit Tests Skipped
| src/filewidgets/kfilewidget.cpp | ||
|---|---|---|
| 1547 ↗ | (On Diff #36499) | Shouldn't this be generalized to all schemes? I.e. if (!u.path().isEmpty() && u.path() != "/") We don't want to turn smb:/// into smb:////. But wait, we also simply don't want to append a slash if there was already one. New suggestion: if (!u.path().isEmpty() && !u.path().endsWith('/')) |
| src/filewidgets/kfilewidget.cpp | ||
|---|---|---|
| 1543 | I think this comment only adds more confusion, because the code doesn't show "" anywhere, it's a side effect of creating an invalid url (or something, I actually wonder why file://// would be invalid...) IMHO it's clear without the added comment. "append slash if needed", and then the if checks if there's already a trailing slash. | |
Comment Actions
Thanks!
| autotests/kfilewidgettest.cpp | ||
|---|---|---|
| 226 ↗ | (On Diff #36533) | This comment should move to line 220, since it matches that specific row of test data ;) |
Comment Actions
The unittest fails in CI, please take a look.
When I tried running the test locally, I got a prompt from sftp about verifying the identity of 127.0.0.1 .... not great for a unittest, to ask for user input.
This also means the test is dependent on kio_sftp being installed? That's no good, it's not part of KIO, so it's not available for CI.
Please use a URL that is sure to work (maybe FTP on a public site), or better, somehow stub out any actual kioslave usage, and *just* test URLs.