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
