KProcess Line Mode Patch

[Andreas Pakulat]

On 02.12.08 16:23:59, Ralf Habacker wrote:
> Andreas Pakulat schrieb:
>> On 02.12.08 13:51:57, Ralf Habacker wrote:
>>> +qint64 KLineBasedReader::readLine(char * data, qint64 maxSize)
>>> +{
>>> +    qint64 ret = m_p->readLine(data, maxSize);
>>> +    if (ret <= 0)
>>> +        return ret;
>>> +    if (data[strlen(data)-2] == '\x0d' && data[strlen(data)-1] == '\x0a')
>>>     
>>
>> That won't happen, QIODevice will already replace windows line-endings with
>> unix-ones (unless you don't use QIODevice::Text, in which case you
>> shouldn't be reading lines).
>>   
> That would mean to override the related QIODevice Setting in  
> KProcessPrivate
>
>        openMode(QIODevice::ReadWrite)

KProcess allows to set any open mode I want, in particular it allows to use
Text (see setNextOpenMode), so if somebody wants to use this class he
should use QIODevice::Text as well - IMHO. Obviously that should be stated
clearly in the api docs.

> <http://lxr.kde.org/source/KDE/kdeadmin/ksystemlog/src/lib/processOutputLogFileReader.cpp#204> 
 
That last one has the encoding problem too. I didn't look closer at the
rest above.

> 358 <http://lxr.kde.org/source/KDE/kdebase/apps/konqueror/settings/konqhtml/pluginopts.cpp#358> void KPluginOptions <http://lxr.kde.org/ident?i=KPluginOptions>::progress <http://lxr.kde.org/ident?i=progress>()

That one could be fixed by simply using setNextOpenMode( QIODevice::Text |
QIODevice::ReadOnly ); 

Anyway, apparently there are a couple of places where splitting a
byte-array into parts via '\n' is used. So, if you can make it provide
QList<QByteArray> instead of char* and emit the signal only if there's a
full new line available as Thiago said I think this could be added for KDE
4.3.

>> Again the encoding problem, readLine returns a QByteArray, you can't just
>> convert that to a QString without knowing the right encoding.
>>   
> This shows once more that there is a need for a such  helper class to  
> avoid broken code

Well, the few places you quoted fully were aware of the encoding issue
(some ignored it on purpose because they don't care). But you can't "fix"
the encoding problem on the library level, only on the application level
because the library cannot know which encoding is used by a given
QIODevice. 

As I said above: The number of occurences of bytearray.split('\n') probably
is enough to warrant for a new class encapsulating that and sending out
signals using QByteArray. Then all the application needs to do is
converting the lines into QString by using the proper encoding.

Andreas

-- 
Don't look back, the lemmings are gaining on you.