D17595: Upstream Dolphin's file rename dialog

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> renamefiledialog.cpp:215
> +    connect(job, &KJob::result, this, &QObject::deleteLater);
> +    connect(job, &KJob::error, this, &RenameFileDialog::slotError);
> +

This can't possibly work, KJob::error is a getter, not a signal.

Remove slotError, and emit error() in slotResult.

> meven wrote in renamefiledialog.cpp:119
> KFileItem has no path()  method, but here items.first().url().fileName() should be equivalent.

Oops yes I meant items.first().url().path(). OK for fileName(), path() is faster but this code path isn't speed critical.

> meven wrote in renamefiledialog.cpp:121
> I guess the database doesn't work for JPEG file extension for instance.

Wrong, QMimeDatabase handles this correctly, extensions are case insensitive.

Proof:
$ kmimetypefinder5 -f foo.JPEG 
image/jpeg

> meven wrote in renamefiledialog.cpp:208
> 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.

Good question, I don't know. I'd say, let the first person who needs it, add it :)

> dfaure wrote in renamefiledialog.cpp:246
> Isn't it enough to focus the lineEdit in the constructor?
> 
> Doing it here in showEvent (without a test for event->spontaneous()) means that if you focus another widget, switch virtual desktops, and switch back, the focus will unexpectedly go to the lineedit again.

I see you used the more complicated solution of keeping showEvent and testing for spontaneous. Any reason? It didn't work from the constructor?

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/5db13757/attachment.htm>