[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Adding support to KMimeType for guessing the
From:       "David Faure" <faure () kde ! org>
Date:       2009-09-28 9:19:44
Message-ID: 20090928091944.32442.59300 () localhost
[Download RAW message or body]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1696/#review2479
-----------------------------------------------------------

Ship it!


Yep, looks ok.

I disagree on removing the QByteArray overloads for kde5 though, they do provide \
convenience (otherwise, everyone calling this will have to create a QBuffer, a bit \
cumbersome since QByteArray *is* the most common use case for this feature)

- David


On 2009-09-23 16:33:18, Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1696/
> -----------------------------------------------------------
> 
> (Updated 2009-09-23 16:33:18)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> KMimeType already supports the detection of the mimetype from data in the working \
> memory. But only using QByteArray as a container class. 
> Now in Okteta, the hex editor from kdeutils, the data is not stored in some \
> continuous memory range, so wrapping it in QByteAray::fromRaw() does not work. And \
> creating a data copy just for mimetype detection does not look like the smartest \
> solution. Especially, as "KMimeType::findByContent( const QByteArray& data )" is \
> just a wrapper to opening a QBuffer buffer on the data object and calling \
> "KMimeTypeFactory::findFromContent( QIODevice* device, ...)", passing the buffer as \
> device. It would be great it KMimeType could do the type guessing for the opened \
> documents in Okteta also based on the content, especially as the data is already \
> locally available (same addressspace, fast working memory), and writing a dedicated \
> (readonly) QIODevice subclass for access is an acceptable overhead IMHO. Others \
> might have similar needs, e.g. with files/data streams stored encoded in some \
> containers and okay to be decoded on-demand by a QIODevice (only a small part of \
> the data might be used, so decoding all of it by default is waste of resources). \
> E.g. Ark might have use for it to detect the mimetype of the files in archives \
> (currently done only based on the filename). 
> Attached patch adds two methods, which overload the two mimetype detection methods \
> using a QByteArray argument as content with QIODevice argument as content provider. \
> For KDE 5 the QByteArray ones might even be removed, being the simple, but limited \
> to continuous data wrappers they are. 
> Okay to commit?
> 
> 
> Diffs
> -----
> 
> trunk/KDE/kdelibs/kdecore/services/kmimetype.h 1025071 
> trunk/KDE/kdelibs/kdecore/services/kmimetype.cpp 1025071 
> 
> Diff: http://reviewboard.kde.org/r/1696/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H.
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]