KProcess Line Mode Patch
apaku at gmx.de
Tue Dec 2 13:23:51 GMT 2008
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
> + QIODevice *m_p;
public API needs to have a d-pointer.
Your boss is a few sandwiches short of a picnic.
More information about the kde-core-devel