D18915: Fix batchrename changing extension to lower case
Ambareesh Balaji
noreply at phabricator.kde.org
Mon Feb 11 03:57:02 GMT 2019
abalaji added a comment.
Added a couple inline comments for you to address. Also, please use four spaces instead of tabs, and keep the spacing consistent (spaces around `+`, space between `if ()` and the `{). I have yet to run and test this, but if you check the comment on the left side of the diff, I wonder if just removing that `.toLower()` is enough.
INLINE COMMENTS
> batchrenamejob.cpp:62
> QSet<QString> extensions;
> - QMimeDatabase db;
> + const QString dot = QString::fromStdString(".");
> foreach (const QUrl &url, m_srcList) {
You can just `QString dot = "."`, but you should use a single character instead of a string, since it's just a single character
> batchrenamejob.cpp:64
> foreach (const QUrl &url, m_srcList) {
> - const QString extension = db.suffixForFileName(url.toDisplayString().toLower());
> if (extensions.contains(extension)) {
I wonder if just getting rid of `.toLower()` fixes this bug
> batchrenamejob.cpp:68
> + //If the oldFileName contains a '.' then its extension is all to the right of the last dot
> + if (urlStr.contains(dot)){
> + extension = urlStr.mid(urlStr.lastIndexOf(dot)+1);
Rather than using `contains` here, you can just use `lastIndexOf`, check if it's not `-1`, and go from there. Just simply `urlStr.lastIndexOf('.')`.
> batchrenamejob.cpp:179
> +
> + const QString dot = QString::fromStdString(".");
> + const QString oldFileName = oldUrl.fileName();
Same as above
> batchrenamejob.cpp:184
> + //If the oldFileName contains a '.' then its extension is all to the right of the last dot
> + if (oldFileName.contains(dot)){
> + extension = oldFileName.mid(oldFileName.lastIndexOf(dot)+1);
Same as above
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D18915
To: cfoster, #dolphin, #frameworks, abalaji
Cc: chinmoyr, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190211/a85d78e4/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list