D17595: Upstream Dolphin's file rename dialog

[Méven Car]

meven marked 4 inline comments as done.
meven added a comment.


  Btw the licensing issue is not resolved yet.
  I will update the license once this has cleared.

INLINE COMMENTS

> dfaure wrote in renamefiledialog.cpp:119
> items.first().path() would be simpler and faster.
> (that doesn't give the same result, but it's sufficient for calling suffixForFileName with it)

KFileItem has no path()  method, but here items.first().url().fileName() should be equivalent.

> dfaure wrote in renamefiledialog.cpp:121
> why toLower()? Should work without.

I guess the database doesn't work for JPEG file extension for instance.

> dfaure wrote in renamefiledialog.cpp:208
> Does Dolphin really want that? I thought a GUI design principle in Dolphin was to avoid messageboxes and show errors in an embedded widget instead.
> 
> So I was expecting this dialog to emit an error signal so that the app can decide on how to handle errors, instead.

Apparently here it did use the default error dialog.
I have changed this to expose an error signal to let the class user handle it.
I wonder if exposing a setAutoErrorHandling setter for this dialog would be a good idea.

> dfaure wrote in renamefiledialog.cpp:60
> You can always revert the changes to the other files ;)
> (commit any other changes, run uncrustify, git commit --amend renamefiledialog.*, git checkout .)
> 
> Or do you mean it would really make the coding style inconsistent with the other files? I would be surprised that the difference would be major.

I just did not bother going through those hoops, but this worked out great. Thanks

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, #dolphin, broulik, ngraham, dfaure
Cc: dfaure, sitter, mitchell, emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190816/598bbba2/attachment.htm>