D22149: Add "Add to Places" action to file menu

[Elvis Angelaccio]

elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


  In D22149#511989 <https://phabricator.kde.org/D22149#511989>, @ngraham wrote:
  
  > In D22149#506093 <https://phabricator.kde.org/D22149#506093>, @elvisangelaccio wrote:
  >
  > > There seems to be another issue: if I run `dolphin <some folder>` from cmd line I always get `Add 'file' to place`, no matter what the folder is.
  >
  >
  > I traced this down to a pre-existing issue in `DolphinViewContainer::url()` returns a URL without the trailing slash stripped, and `url().fileName()` as called in `DolphinViewContainer::placesText()` returns an empty string if a trailing slash in the URL (this is a documented behavior in `QUrl::fileName()`: https://doc.qt.io/qt-5/qurl.html#fileName). When you navigate around Dolphin normally, the URL returned by `url()` has its trailing slash stripped already, but when you open Dolphin with a path, the trailing slash is not stripped, so the behavior is different and the text is wrong.
  >
  > I think this should be fixed in another patch because it is a pre-existing issue. I would have submitted a patch to fix it already, but I wanted to discuss with you where you think the appropriate fix should be made. `DolphinViewContainer::url()` could always strip the trailing slash before returning the URL, or perhaps command-line input could have its trailing slash stripped before being passed around in Dolphin. Or both, for safety. What do you think?
  
  
  I'm a bit scared to strip trailing slashes from the `url()` of the view.
  
  I think we should fix `placesText()` instead, see D23654 <https://phabricator.kde.org/D23654>.
  
  > Either way I think this patch is ready to land. We can do it now, or else we can wait for the other issue to be fixed first.
  
  Yes, feel freel to land it.

INLINE COMMENTS

> dolphinui.rc:2
>  <!DOCTYPE kpartgui SYSTEM "kpartgui.dtd">
> -<kpartgui name="dolphin" version="22">
> +<kpartgui name="dolphin" version="23">
>      <MenuBar>

Needs rebase.

REPOSITORY
  R318 Dolphin

BRANCH
  arcpatch-D22149

REVISION DETAIL
  https://phabricator.kde.org/D22149

To: ngraham, #dolphin, elvisangelaccio
Cc: elvisangelaccio, kfm-devel, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190901/e333d15c/attachment.htm>