KProcess Line Mode Patch

[Andreas Pakulat]

On 02.12.08 13:51:57, Ralf Habacker wrote:
> Appended is a proposal for such a class working with the  
> QProcess/KProcess output channel.
>
> One question I have is where in the kdelibs such a class could be located ?
..
> +#include <QtDebug>

That is probably not wanted in this code.

> +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).

> +        data[strlen(data)-2] = '\0';
> +    else if (data[strlen(data)-1] == '\x0a')
> +        data[strlen(data)-1] = '\0';
> +    return ret;
> +}

With the above changes, what exactly is the benefit of this class? IMHO it
doesn't provide any real value compared to listening for the readyRead()
signal from QIODevice+checking for canReadLine and then reading the line
into a char* or even a QByteArray.

So where are the two apps that use this class? And why do they use it?

> +QString KLineBasedReader::readLine (qint64 maxSize)
> +{
> +    QString data = m_p->readLine(maxSize);

Again the encoding problem, readLine returns a QByteArray, you can't just
convert that to a QString without knowing the right encoding.

> +#include <QObject>

This needs to be QtCore/QObject

> +private Q_SLOTS:
> +    void slotReadyRead(); 

This should use the Q_PRIVATE_SLOT macro

> +private: 
> +    QIODevice *m_p;

public API needs to have a d-pointer.

Andreas

-- 
Your boss is a few sandwiches short of a picnic.