[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kfile
From:       David Faure <faure () kde ! org>
Date:       2011-01-10 18:21:35
Message-ID: 201101101921.36367.faure () kde ! org
[Download RAW message or body]

On Monday 10 January 2011, André Wöbbeking wrote:
> On Monday 10 January 2011, Sebastian Trueg wrote:
> > SVN commit 1213464 by trueg:
> > 
> > When sorting by name KDirSortFilterProxyModel uses UDS_DISPLAY_NAME.
> > The latter, however, is not unique. This results in strange GUI behaviour
> > like swapping items. This patch makes the model fall back to UDS_NAME to
> > ensure a fixed sort order.
> > 
> > See also review request http://reviewboard.kde.org/r/6322/
> > 
> >  M  +10 -1     kdirsortfilterproxymodel.cpp
> > 
> > --- trunk/KDE/kdelibs/kfile/kdirsortfilterproxymodel.cpp #1213463:1213464
> > @@ -168,8 +168,17 @@
> > 
> >      switch (left.column()) {
> >      case KDirModel::Name: {
> > 
> > -        return d->compare(leftFileItem.text(), rightFileItem.text(),
> > sortCaseSensitivity()) < 0; +        // KFileItem::text() may not be
> > unique (in case UDS_DISPLAY_NAME is used). In that case we +        //
> > fall back to the name which is always unique
> > +        const int result = d->compare(leftFileItem.text(),
> > rightFileItem.text(), sortCaseSensitivity()); +        if (result == 0) {
> > +            return d->compare(leftFileItem.name(sortCaseSensitivity() ==
> > Qt::CaseInsensitive), +
> > rightFileItem.name(sortCaseSensitivity() == Qt::CaseInsensitive),
> > +                           sortCaseSensitivity()) < 0;
> 
> Why do you call name() with sortCaseSensitivity() == Qt::CaseInsensitive
> when you already have the case sensitivity in compare()?

To benefit from the lowercase-name caching done inside KFileItem::name(bool), I 
suppose.

Although then the last argument to compare() might not be necessary. Currently 
there is no benefit indeed, since a case-insensitive comparison will be done 
anyway.

-- 
David Faure, faure@kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).

[prev in list] [next in list] [prev in thread] [next in thread]