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

List:       kde-commits
Subject:    Re: KDE/kdelibs/kfile
From:       André_Wöbbeking <Woebbeking () kde ! org>
Date:       2011-01-10 19:21:14
Message-ID: 201101102021.15518.Woebbeking () kde ! org
[Download RAW message or body]

On Monday 10 January 2011, David Faure wrote:
> 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.

Ah, I didn't know about lowercase-name caching inside KFileItem but

compare(leftFileItem.name(), rightFileItem.name(), sortCaseSensitivity()) < 0

is of course better readable too.

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