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

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=15229


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.

Fixed.

> * 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.

Fixed.

> * 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
too.

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

Fixed.

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