D9662: folderspanel context-menu option "Limit to Home Directory" should be always visible

Michael Heidelbach noreply at phabricator.kde.org
Mon Jan 15 11:01:35 GMT 2018


michaelh marked 2 inline comments as done.
michaelh added a comment.


  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

INLINE COMMENTS

> ngraham wrote in CMakeLists.txt:6
> Don't touch this; the release process manages it.

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)" :)

> elvisangelaccio wrote in folderspanel.cpp:330
> While at it, use `QStringLiteral()`

Should this have the same effect as

  baseUrl = QUrl::fromLocalFile(QDir::rootPath());

I would like to refactor this if..block a little bit.

> elvisangelaccio wrote in folderspanel.h:94
> Bools in APIs are bad for readability, better use enums (e.g. `loadUrl(url, FoldersPanel::AllowJumpHome)`).

Please explain this a little bit. Because by looking at this code I can see no advantage.

REPOSITORY
  R318 Dolphin

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

To: michaelh, #dolphin, emmanuelp, ngraham
Cc: elvisangelaccio, michaelh, spoorun, navarromorales, isidorov, firef, ngraham, andrebarros, alexeymin, genaxxx, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180115/de2c6ad6/attachment.htm>


More information about the kfm-devel mailing list