Pushed new version 1

Thinking about it, maybe this should be renamed to
static QString htmlEscape(const QString& plain);
in the QString class,

and Qt::escape could stay in QtGui for full source compatibility, and be marked as deprecated (and be implemented with a call to QString::htmlEscape). Opinions?

i'm +0.5 on renaming it.
i don’t think it would make any sense to leave the alias in gui.

fwiw, the renaming can/should be a separate commit/MR.

Make Qt::escape an inline QT4_COMPAT method in qstring.h (no export), calling the htmlEscape function.

Shouldn’t be called xmlEscape? Or sgmlEscape?

I think it also makes sense to move convertFromPlainText and codecForHtml methods since both handle QtCore types.
The mightBeRichText method would also be a candidate, if not using QTextHtmlParser internally.

Thanks for the comments.
I think htmlEscape is the better name, given that the most common use case for it is to convert plain text for use with Qt’s rendering.

I moved convertFromPlainText too, renaming it plainTextToHtml (to make clear what conversion this is really about).

About Qt::codecForHtml : it’s not documented, it’s already a compat method for QTextCodec::codecForHtml.

Pushed new version 2

Well, look at how the code uses this method:

rich += QString::plainTextToHtml(QLatin1String(m), Qt::WhiteSpaceNormal);

This is much like a fromFoo() method.
With your suggestion it would become

QString text = QLatin1String(m);
rich += text.autoFormattedToHtml(Qt::WhiteSpaceNormal);

I'm not sure that’s really better.

Pushed new version 3

I fixed the attached braces “){”, but what did you mean by “missing braces”? I usually put too many of them :–)

Indentation fixed as well. This leaves the issue of static or non-static methods.

QString::htmlEscape looks fine to me. Maybe it should be a member function, though (like QByteArray::toPercentEncoding). Hence named QString::toHtmlEscaped.

QString::plainTextToHtml sounds a lot more involved to me. I don’t want qstring.cpp to be the dumping ground of every single string operation that might happen. Maybe it’s time to create a class for string operations and utilities? There are other useful candidates, like checking if a given byte array is properly UTF-8 encoded.

(FTR: i hate gitorious …)

@thiago: well, in this case this “misc” class would become a dumping ground. we have/had that in kde already …

maybe it should be a “topically bound” class like QHtmlUtils or QRichTextUtils.

I actually don’t care much for plainTextToHtml. Let’s keep it out of this then, if it’s not clear what it should become?

Pushed new version 4

r=me

Is it better to optimize cycle like that:

for (const_iterator i = begin(), e = end(); i != e; ++i) {
    switch(i->toLatin1()) {  
    case '<':  
       //...  
    }  
}

Because currently it is very pessimistic – calls QString::at() generally 5 times per iteration. I know, at() is very fast. But it should contain some checks (but I do not exam source code), which can slow down operation.

In release mode there are no checks in at().

inline const QChar QString::at(int i) const
{ Q_ASSERT(uint(i) < uint(size())); return d->data()[i]; }

But indeed a switch is a good idea.

Trying… interesting, libQtCore.so is now 264 bytes bigger, with this additional patch: http://www.davidfaure.fr/kde/switch.diff
And in release mode, callgrind says the number of cpu instructions spent in toHtmlEscaped() increased!
I admit that I don’t understand why, the only way to dig further would be to read assembly code, since kcachegrind says it’s the number of instructions spent in toHtmlEscaped itself that changed (the calls to QString::append/realloc/grow are exactly the same in both cases).

Maybe toLatin1() is more expensive than comparisons between QChar and QLatin1Char(‘x’) which is optimized out.

You can try use QChar::unicode() instead of toLatin1() and compare with L'<‘… But I don’t think it is good idea…
Under Linux it should work…

Thanks!

Merged in:
e316deffef68287b03f6eed6e98b5bfcd984280d
c7a0fd0950179ed9ae0aa1e5b02df9b6a383c709