From kde-panel-devel Thu Apr 30 09:11:48 2015 From: "Ashish Bansal" Date: Thu, 30 Apr 2015 09:11:48 +0000 To: kde-panel-devel Subject: Re: Review Request 123224: KIO::suggestName suggests wrong name for some filenames Message-Id: <20150430091148.21733.51209 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=143038514700731 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0905957555729741643==" --===============0905957555729741643== Content-Type: multipart/alternative; boundary="===============4456311212908274418==" --===============4456311212908274418== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On April 28, 2015, 2:35 p.m., David Faure wrote: > > src/core/global.cpp, line 407 > > > > > > startsWith('.') (using the QChar overload) > > > > Do we even need this special case, with the way the code is now? It seems to me that removing this first if() branch would work just the same. > > Ashish Bansal wrote: > The first if() has been used for getting ". (1).txt" instead of ".txt (1)" But yeah, code looks nasty with this special case. Should I remove this? - Ashish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123224/#review79629 ----------------------------------------------------------- On April 30, 2015, 9:05 a.m., Ashish Bansal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123224/ > ----------------------------------------------------------- > > (Updated April 30, 2015, 9:05 a.m.) > > > Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Arjun AK. > > > Bugs: 341773 > https://bugs.kde.org/show_bug.cgi?id=341773 > > > Repository: kio > > > Description > ------- > > For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong name(something like filename-1 2.6.tar.gz). > Expected name: filename-1.6 (1).tar.gz > > > Diffs > ----- > > autotests/globaltest.cpp ff8725d > src/core/global.cpp f18ac10 > > Diff: https://git.reviewboard.kde.org/r/123224/diff/ > > > Testing > ------- > > Works fine! > > > Thanks, > > Ashish Bansal > > --===============4456311212908274418== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123224/

On April 28th, 2015, 2:35 p.m. UTC, David Faure wrote:

src/core/global.cpp (Diff revision 3)
QUrl KIO::upUrl(const QUrl &url)
407	if (index != -1) {	396	if (oldName.startsWith(".") && oldName.count(".") == 1) {

startsWith('.')  (using the QChar overload)
Do we even need this special case, with the way the code is now? It seems to me that removing this first if() branch would work just the same.

On April 30th, 2015, 9:06 a.m. UTC, Ashish Bansal wrote:

The first if() has been used for getting ". (1).txt" instead of ".txt (1)"

But yeah, code looks nasty with this special case. Should I remove this?

- Ashish

On April 30th, 2015, 9:05 a.m. UTC, Ashish Bansal wrote:

Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Arjun AK.

By Ashish Bansal.

Updated April 30, 2015, 9:05 a.m.

Bugs: 341773

Repository: kio

Description

For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong name(something like filename-1 2.6.tar.gz).
Expected name: filename-1.6 (1).tar.gz

Testing

Works fine!

Diffs

autotests/globaltest.cpp (ff8725d)
src/core/global.cpp (f18ac10)

--===============4456311212908274418==-- --===============0905957555729741643== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo= --===============0905957555729741643==--