Candidate commit: "white-space: pre"

[Leo Savernik]

Am Montag, 27. Dezember 2004 19:27 schrieb Allan Sandfeld Jensen:
> Hi everybody and "glædelig jul"

You're Swedish? I've always thought you were an US inhabitant.
>
> I've attached my candidate commit patch for CSS "white-space:pre".

Yay! Good to finally have a reliable white-space: pre support.

> All major regressions and problems have been eliminated, and only a few
> minor issues remain. So I would ask for your oppion before commiting.

I've reviewed your patch. See below.
>
[...]

> @@ -352,8 +353,7 @@ bool KHTMLParser::insertNode(NodeImpl *n
>         // never create anonymous objects just to hold a space.
>         if ( id == ID_TEXT &&
>              static_cast<TextImpl *>(n)->string()->l == 1 &&
> -            (static_cast<TextImpl *>(n)->string()->s[0] == ' ' ||
> -              static_cast<TextImpl *>(n)->string()->s[0] == '\n'))
> +             static_cast<TextImpl *>(n)->string()->s[0] == " " )

Is the usage of a char * instead of a char intentional?

[...]
> -inline int closeWordAndGetWidth(const QFontMetrics &fm, const QChar *str, 
int pos,
> +inline int closeWordAndGetWidth(const QFontMetrics &fm, const QString str, 
int pos,

Passing a QString, and that one const? Didn't you forget the reference?

>         int wordStart, int wordEnd)
>  {
>      if (wordEnd <= wordStart) return 0;
>  
> -    QConstString s(str + pos + wordStart, wordEnd - wordStart);
> -    return fm.width(s.string());
> +    //QConstString s(str + pos + wordStart, wordEnd - wordStart);
> +    const QString s = str.mid(pos + wordStart, wordEnd - wordStart);
> +    return fm.width(s);

Can you elaborate on that change, please?

>  }
>  
>  /** closes the current word and draws it
> @@ -76,7 +79,7 @@ inline int closeWordAndGetWidth(const QF
>   * @param wordEnd relative index pointing one position after the word ended
>   */
>  inline void closeAndDrawWord(QPainter *p, QPainter::TextDirection d,
> -       int &x, int y, const short widths[], const QChar *str, int pos,
> +       int &x, int y, const short widths[], const QString str, int pos,
>         int &wordStart, int wordEnd)

Same here.

>  {
>      if (wordEnd <= wordStart) return;
> @@ -85,8 +88,9 @@ inline void closeAndDrawWord(QPainter *p
>      if (d == QPainter::RTL)
>        x -= width;
>  
> -    QConstString s(str + pos + wordStart, wordEnd - wordStart);
> -    p->drawText(x, y, s.string(), -1, d);
> +    //QConstString s(str + pos + wordStart, wordEnd - wordStart);
> +    const QString s = str.mid(pos + wordStart, wordEnd - wordStart);

Same here

> +    p->drawText(x, y, s, -1, d);
>  
>      if (d != QPainter::RTL)
>        x += width;
> @@ -95,12 +99,17 @@ inline void closeAndDrawWord(QPainter *p
>  }
>  
>  void Font::drawText( QPainter *p, int x, int y, QChar *str, int slen, int 
pos, int len,
> -        int toAdd, QPainter::TextDirection d, int from, int to, QColor bg, 
int uy, int h, int deco ) const
> +        int toAdd, QPainter::TextDirection d, int from, int to, QColor bg, 
int uy, int h,
> +        int deco ) const
>  {
>      if (!str) return;
>      QConstString cstr = QConstString(str, slen);
>      QString qstr = cstr.string();
>  
> +    for(int i=pos; i<pos+len; i++) 
> +        if (qstr[i].unicode() == '\n')
> +            qstr[i] = QChar(' ');
> +
>      // ### fixme for RTL
>      if ( !scFont && !letterSpacing && !wordSpacing && !toAdd && from==-1 ) 
{
>         // simply draw it
> @@ -112,7 +121,7 @@ void Font::drawText( QPainter *p, int x,
>         int numSpaces = 0;
>         if ( toAdd ) {
>             for( int i = 0; i < len; ++i )
> -               if ( str[i+pos].category() == QChar::Separator_Space )
> +               if ( qstr[i+pos].category() == QChar::Separator_Space )
>                     ++numSpaces;
>         }
>  
> @@ -144,17 +153,19 @@ void Font::drawText( QPainter *p, int x,
>         if (mode == Whole) {    // most likely variant is treated extra
>  
>             if (to < 0) to = len;
> -           const QConstString cstr(str + pos, len);
> -           const QConstString segStr(str + pos + from, to - from);
> -           const int preSegmentWidth = fm.width(cstr.string(), from);
> -           const int segmentWidth = fm.width(segStr.string());
> +           //const QConstString cstr(str + pos, len);
> +            const QString cstr = qstr.mid(pos,len);

Again, why?

> +           //const QConstString segStr(str + pos + from, to - from);
> +            const QString segStr = qstr.mid(pos+from, to-from);
> +            const int preSegmentWidth = fm.width(cstr, from);
> +           const int segmentWidth = fm.width(segStr);
>             const int eff_x = d == QPainter::RTL ? x - preSegmentWidth - 
segmentWidth
>                                         : x + preSegmentWidth;
>  
>             // draw whole string segment, with optional background
>             if ( bg.isValid() )
>                 p->fillRect( eff_x, uy, segmentWidth, h, bg );
> -           p->drawText(eff_x, y, segStr.string(), -1, d);
> +           p->drawText(eff_x, y, segStr, -1, d);
>             if (deco)
>                 drawDecoration(p, eff_x, uy, y - uy, segmentWidth - 1, h, 
deco);
>             return;
[...]

Otherwise, your patch looks good.

mfg
 Leo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20041227/c5956c4d/attachment.sig>