----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101075/#review2553 -----------------------------------------------------------
While I'm not really qualified to comment on the implementation, I agree that this is a much needed feature. Just last week, I was trying to use "all/allfiles" in KFileDialog and was really quite surprised to learn that it didn't actually match ANY files. - Parker On April 9, 2011, 10:17 p.m., Thomas Fischer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101075/ > ----------------------------------------------------------- > > (Updated April 9, 2011, 10:17 p.m.) > > > Review request for kdelibs. > > > Summary > ------- > > 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. > > > Diffs > ----- > > kfile/kfilefiltercombo.cpp f32a0df > kio/kio/kdirlister.cpp df81dc8 > > Diff: http://git.reviewboard.kde.org/r/101075/diff > > > Testing > ------- > > > Thanks, > > Thomas > >
