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