D8208: Add Duplicate feature

[David Faure]

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

INLINE COMMENTS

> dolphinview.cpp:721
> +        QString fileLocation = originalPath;
> +        fileLocation.remove(originalFileName);
> +

This will behave wrong on /home/dfaure/Documents/dfaure
(it will remove the first dfaure instead of the second)
Substring search done by remove() is both dangerous and slow.

You could just do

  const QString fileLocation = originalURL.path(QUrl::RemoveFilename);

(I'd call it directoryPath btw, in QUrl terms it's a path)

> dolphinview.cpp:726
> +        // Append "copy" to the end of the file name, but before the suffix, if present
> +        const QMimeDatabase db;
> +        const QString extension = db.suffixForFileName(originalFileName);

move it out of the loop

> dolphinview.cpp:737
> +            const QString suffixString = QStringLiteral(".") + originalFileName.right(extension.size());
> +            const QString originalFilenameWithoutExtension = originalFileName.chopped(extension.size() + 1); // account for the dot
> +            duplicateURL.setPath(i18nc("<file path><filename> copy.<extension>", "%1%2 copy%3", fileLocation, originalFilenameWithoutExtension, suffixString));

It would be simpler to keep the dot here (then both this line and the previous one become simpler).

REPOSITORY
  R318 Dolphin

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

To: ngraham, #dolphin, #kde_applications, elvisangelaccio, dfaure, broulik, davidedmundson
Cc: fvogt, mthw, kfm-devel, meven, markg, fazevedo, cfeck, #dolphin, pberestov, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200111/d75b1e09/attachment.htm>