<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/118128/">https://git.reviewboard.kde.org/r/118128/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 14th, 2014, 2:24 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On May 14th, 2014, 2:41 p.m. UTC, <b>Frank Reininghaus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
</blockquote>
<p>On May 14th, 2014, 3 p.m. UTC, <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On May 14th, 2014, 3:16 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 :-)
</pre>
</blockquote>
<p>On May 14th, 2014, 3:19 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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....</pre>
</blockquote>
<p>On May 14th, 2014, 3:25 p.m. UTC, <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Any progress on this? It looks like the concensus is to not do any formatting on the model side. Can you upload a patch that does that?</pre>
<br />
<p>- Alex</p>
<br />
<p>On May 14th, 2014, 2:01 p.m. UTC, Martin Klapetek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and David Faure.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated May 14, 2014, 2:01 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/widgets/kdirmodel.cpp <span style="color: grey">(70d5ee4)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118128/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>