<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="http://git.reviewboard.kde.org/r/101075/">http://git.reviewboard.kde.org/r/101075/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Thomas Fischer.</div>
<p style="color: grey;"><i>Updated April 20, 2011, 8:50 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">I updated my previous patch in two aspects.
(1) The code formatting has been changed to match the surrounding code.
(2) The documentation for kfiledialog.h has been rewritten to match the changes. See function setFilter() for the main changes. Across the documentation, the variants of "mimetype" and "mime-type" have been unified.</pre>
</td>
</tr>
</table>
<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;">When using mimetypes instead of old-fashioned '*.txt|Text file' filters for KFileDialog functions, there is no mimetype working like '*|All files'. Mimetypes 'all/all' and 'all/allfiles' have no globs associated and as such do not match any filenames. The current solution 'application/octet-stream' works as an all-files filter, but does not behave nicely in KFileFilterCombo: including this mimetype will render 'all supported files' to 'all files'.
The attached patch improves the situation as follows: if the developer adds a mimetype 'all/allfiles' to his/her list of mime types for filtering in e.g. KFileDialog::getOpenUrl's second parameter, function setMimeFilter recognizes the request for an "all files" filter option and adds it to the list of options in the combobox. This "all files", however, is not added to the "all support files" option as the 'application/octet-stream' would have been. Additional minor changes (moving some lines around "+= delim") were made in the same function for GUI consistency reasons.
To make the patch and mimetype 'all/allfiles' working (as it has no inherent glob like '*'), KDirLister has to be modified as well: just like 'application/octet-stream' can act as a wildcard, so does 'all/allfiles' now.
Regarding code consistency, I am not sure why 'application/octet-stream' was used as a filter; this mimetype stands IMHO for binary bulk data instead of 'any/all files'. Maybe support for application/octet-stream should be marked as deprecated in this situation.
If this patch does get accepted, the documentation on setMimeFilter needs to be adopted to explain the use of 'all/allfiles'. Furthermore, I would suggest that the documentation should suggest to prefer using mimetypes ('text/plain') over manual globs ('*.txt|Text file') as it offers better consistency ('*.txt|Text file' vs '*.txt|Plain Text file'), internationalization etc.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kfile/kfilefiltercombo.cpp <span style="color: grey">(f32a0df)</span></li>
<li>kio/kfile/kfiledialog.h <span style="color: grey">(af29a37)</span></li>
<li>kio/kio/kdirlister.cpp <span style="color: grey">(df81dc8)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101075/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>