Review Request 123224: KIO::suggestName suggests wrong name for some filenames

[David Faure]

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



src/core/global.cpp (line 396)
<https://git.reviewboard.kde.org/r/123224/#comment54440>

    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.



src/core/global.cpp (line 398)
<https://git.reviewboard.kde.org/r/123224/#comment54437>

    oldName.mid(1), faster (and more readable) than section with an empty separator.



src/core/global.cpp (line 399)
<https://git.reviewboard.kde.org/r/123224/#comment54435>

    isEmpty() rather than isNull(), not point in being specific about the difference between the two.



src/core/global.cpp (line 402)
<https://git.reviewboard.kde.org/r/123224/#comment54436>

    Faster: nameSuffix.prepend('.')  (using the QChar overload).



src/core/global.cpp (line 403)
<https://git.reviewboard.kde.org/r/123224/#comment54442>

    oldName.left(...) or .mid(0, ...)
    ... I'm not even sure what section(empty string, ...) really does :-)



src/core/global.cpp (line 414)
<https://git.reviewboard.kde.org/r/123224/#comment54443>

    does not exists -> does not exist



src/core/global.cpp (line 415)
<https://git.reviewboard.kde.org/r/123224/#comment54444>

    basename +=



src/core/global.cpp (line 417)
<https://git.reviewboard.kde.org/r/123224/#comment54445>

    QString suggestedName = ....
    
    (it's not used before this line, so it should be declared here; could even be "const QString suggestedName = ..." since it's not modified later)


- David Faure


On April 26, 2015, 12:19 p.m., Ashish Bansal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123224/
> -----------------------------------------------------------
> 
> (Updated April 26, 2015, 12:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Aleix Pol Gonzalez.
> 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150428/93021826/attachment-0001.html>