D17595: Upstream Dolphin's file rename dialog

Stefan BrĂ¼ns noreply at phabricator.kde.org
Thu May 9 13:47:44 BST 2019


bruns added inline comments.

INLINE COMMENTS

> renamefiledialog.cpp:60
> +               i18nc("@title:window", "Rename Items"));
> +    QDialogButtonBox *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok|QDialogButtonBox::Cancel);
> +    QVBoxLayout *mainLayout = new QVBoxLayout;

space around `|`

> renamefiledialog.cpp:81
> +    if (m_renameOneItem) {
> +        m_newName = items.first().name();
> +        editLabel = new QLabel(xi18nc("@label:textbox", "Rename the item <filename>%1</filename> to:", m_newName),

`m_newName` is only used in the constructor, no need for it to be a member

> renamefiledialog.cpp:101
> +        QMimeDatabase db;
> +        const QString extension = db.suffixForFileName(fileName.toLower());
> +

is this correct for e.g. `foo.jpeg`? - the preferred extension is `jpg`, so this may select to many characters

> renamefiledialog.cpp:104
> +        // If the current item is a directory, select the whole file name.
> +        if ((extension.length() > 0) && !items.first().isDir()) {
> +            // Don't select the extension

the `!isDir()` should apply to the whole block, i.e. create the QMimeDatabase instance only if needed

> renamefiledialog.cpp:121
> +        QSet<QString> extensions;
> +        foreach (const KFileItem& item, m_items) {
> +            QMimeDatabase db;

`qAsConst(m_items)`

> renamefiledialog.cpp:122
> +        foreach (const KFileItem& item, m_items) {
> +            QMimeDatabase db;
> +            const QString extension = db.suffixForFileName(item.url().toDisplayString().toLower());

move db instance out of loop

> renamefiledialog.cpp:126
> +            if (extensions.contains(extension)) {
> +                m_allExtensionsDifferent = false;
> +                break;

better name - `m_extensionsAreUnique`

> renamefiledialog.cpp:174
> +        job = KIO::moveAs(oldUrl, newUrl, KIO::HideProgressInfo);
> +    } else {
> +        cmdType = KIO::FileUndoManager::BatchRename;

`m_renamedItems.reserve(m_items.count())` in this block

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, broulik
Cc: bruns, meven, dhaumann, pino, kde-frameworks-devel, michaelh, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190509/37909fe6/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list