BUG: 389135
Currently dolphin shows a folder's name instead of a "place" name if the current path has trailing slash and place's name doesn't, or vice versa.
This patch fixes this behavior, so that dolphin always shows a "place" name.
elvisangelaccio |
Dolphin |
BUG: 389135
Currently dolphin shows a folder's name instead of a "place" name if the current path has trailing slash and place's name doesn't, or vice versa.
This patch fixes this behavior, so that dolphin always shows a "place" name.
Test configuration: https://imgur.com/a/U4zBp8c
Before (wrong window titles are in red): https://imgur.com/a/dB4xRwo
After (all window titles are correct): https://imgur.com/a/W7virBn
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
As far as I understand, nope, that commit just changes the url which is going to be added to "places".
Firstly, it doesn't fix the bug, and secondly, a user still can add/modify a url in places so that it contains a trailing slash, and current version of dolphin will show it's name wrong (if opened manually, as on the screenshots in "Test Plan").
In that case it would seem like a simpler approach might be to just have url() always strip the trailing slash, rather than adding a bunch of complex code to detect, parse, and handle it.
Yep, that's exactly what I did to fix that the first time I tried to write a patch!
The problem is, it doesn't solve it.
It will show the name of a folder wrong, if its url in places contains trailing slash (and it's easy to do, the patch that you linked still allows users to manually create/edit such urls in "places").
(The version which just always strips the trailing slash: https://i.imgur.com/QR0GGnx.png)
Please change line 459 to use the following code instead:
const auto& matchedPlaces = placesModel->match(placesModel->index(0,0), KFilePlacesModel::UrlRole, url().adjusted(QUrl::StripTrailingSlash), 1, Qt::MatchStartsWith);
i.e. always strip the trailing slash and check whether the place URL starts with the stripped URL.
Updated!
I didn't think about this solution, thanks, clearly it's way better than mine.
Actually this introduces a regression, sorry about that. dolphin / will now show "Home" as window title...
Will have to think more about it. Maybe some regex could help us.
Oh, and you are right again.
The previous solution seems to fix it without a regression, but obviously the code itself feels at the very least "not good".
What about something like this?
const auto& matchedPlaces = placesModel->match(placesModel->index(0,0), KFilePlacesModel::UrlRole, QUrl(url().adjusted(QUrl::StripTrailingSlash).toString().append("/?")), 1, Qt::MatchRegExp);
I did a few tests, and it seems like it does what it is supposed to do.
Nice, this seems to do the trick :)
src/dolphinviewcontainer.cpp | ||
---|---|---|
459 | We should probably use toString(QUrl::FullyEncoded), since this is how the URL is going to end up in the bookmark XML file used by the places model. |