> On May 14, 2014, 4: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 :-) > > > > David Faure wrote: > 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....
Yeah, another way is to make it return just bytes and use the KFormat on the UI side, that also makes sense. As for the visual change - Plasma folder view's tooltip, which shows file sizes, currently in bytes (folderview is based on kdirmodel) - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118128/#review57923 ----------------------------------------------------------- On May 14, 2014, 4: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, 4: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
