[Webkit-unassigned] [Bug 15229] [gtk] abstract font management by using pango

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 21 08:03:53 PDT 2007


sven at imendio.com changed:

           What    |Removed                     |Added
  Attachment #16339|review?                     |
               Flag|                            |
  Attachment #16339|0                           |1
        is obsolete|                            |
  Attachment #16340|                            |review?
               Flag|                            |

------- Comment #4 from sven at imendio.com  2007-09-21 08:03 PDT -------
Created an attachment (id=16340)
 --> (http://bugs.webkit.org/attachment.cgi?id=16340&action=view)
Proposed Patch v2

(In reply to comment #3)
> * Many newly-added function calls have unneeded whitespace between the function
> name and the opening parenthesis.


> * There are several places where you have extra whitespace around, or incorrect
> placement of, the * in declarations of pointer variables.  In several places
> you have extra whitespace around the = operator in assignments.  More
> generally, we don't typically use whitespace to line up assignments and
> declarations.


> * In several places you have broken function calls across multiple lines for no
> apparent reason.  In cases where it's wrapped due to one argument being the
> result of a function call, it may be preferable to use a local variable to
> simplify the code rather than jamming it all into the function call.  In other
> cases, having all the arguments on the same line may be preferable.

Fixed. I just didn't fix this for two calls:
FontPlatformDataGdk.cpp: FontPlatformData::init(),
GlyphPageTreeNodeGdk.cpp: GlyphPage::fill() - introducing a local variable will
require adding braces - which in turn will lead up to code that looks almost as
scrammed as it does now.

> * When checking if a pointer is NULL, !theVariable is preferred over
> theVariable == 0.

I haven't done that, it was a plain int that I compared to, but this is fixed

> * Declaring local variables at the point of first use is preferred over
> declaring at the top of a function then initializing later.


> Sorry for being so picky!  Hopefully someone can review the real meat of your
> patch soon.

Well, thanks for the review.

Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

More information about the webkit-unassigned mailing list