> On May 14, 2014, 2:24 p.m., David Faure wrote: > > It is correct that this is about a string representation of the filesize, > > to displaying in a column of the model. > > For machine processing one can use KFileItem::size() after getting the > > KFileItem out of the KDirModel. > > > > However, do we really want that the detailed listview in dolphin? I can > > more easily recognize the biggest number by being the one with most digits, > > than having to go through a list of kB, MB, and GB values. > > > > It might even break the sorting by size. > > > > Let's ask Frank Reininghaus, but I'm not sure this is a good idea. > > Frank Reininghaus wrote: > I might not have fully understood what exactly this patch will change in > the GUI - both the file dialog (which uses KDirModel) and Dolphin already do > show "B/KiB/MiB/etc." values in the detailed view here (on a system with the > rather old KDE SC 4.10.5 - I currently don't have anything else to test > here), but maybe something changed about that in frameworks? > > If QSortFilterProxyModel uses the value returned by the function that is > modified by this patch (I think it does?), then this might really break > sorting by size in the dialog. Have you checked this, Martin? > > Martin Klapetek wrote: > Frank - what this changes is returning bytes formatted by your locale to > returning the human readable size + the unit itself. So for example a file > with 20000000 bytes would return "20,000,000" for locales with "," as > thousands delimiter before this patch, whereas now it would return "20 MB" > (including the unit). I'm not sure if Dolphin even uses this though; I also > think this already does not work with sorting (as also Mark G. noted) because > it does not do locale-aware sort. > > David - well if your file is 2487651687 bytes big, isn't it just easier > to read that as 2.3 GiB? ;) Plus, Dolphin indeed already does this. > > > -- > I just think that it's better to actually return a locale-formatted > human-readable number rather than just locale-formatted number, which is what > this patch does :) Unless this should break things... > > Alternatively, if this is actually wanted, I can add another role like > Mark suggested. > > David Faure wrote: > OK, first step, then is: figuring out why sorting works in KFileDialog > right now in kdelibs4, and where the user-readable strings come from. > KDirModel, also in kdelibs4, only returns a locale-formatted number, not > a human-readable one. > > Looks like my choice in KDirModel got overriden in the layers above it > anyway :-) > >
Ah, it's done in the delegate, see KFileItemDelegate::Private::itemSize(). So actually KDirModel should just return a plain number, for sorting to work out of the box :-) Now I'm curious, Martin, where did this patch make any visual change? Surely not in anything that uses KFileItemDelegate.... - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118128/#review57923 ----------------------------------------------------------- On May 14, 2014, 2:01 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118128/ > ----------------------------------------------------------- > > (Updated May 14, 2014, 2:01 p.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kio > > > Description > ------- > > Now I'm not sure if returning the unit together with the size won't break > some things, but as it returns string already, I thought returning it in the > "human readable" form would be better than always returning bytes. > > > Diffs > ----- > > src/widgets/kdirmodel.cpp 70d5ee4 > > Diff: https://git.reviewboard.kde.org/r/118128/diff/ > > > Testing > ------- > > > Thanks, > > Martin Klapetek > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel