[Webkit-unassigned] [Bug 8515] Linux porting compile bug

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=8515


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #8761|review?                     |review-
               Flag|                            |




------- Comment #19 from darin at apple.com  2006-06-08 09:13 PDT -------
(From update of attachment 8761)
The .cpp files are missing includes of "config.h". Every .cpp file needs to
include "config.h". Also, each .cpp file should includes its own .h file first,
right after "config.h".

I don't think the file "PlatformKeyboardCodes.h" should have the name
"Platform" in its name. The only reason the event files have Platform in their
name is that they conflict with DOM classes. I think "KeyboardCodes.h" would be
a fine name for the file.

Headers must not include "config.h". That's included by every .cpp file
instead.

    return IntPoint(contentsPoint) + scrollOffset();

Why the cast to IntPoint here?

    m_isKeyUp = bool( event->type == GDK_KEY_RELEASE);
    m_shiftKey = bool(event->key.state & GDK_SHIFT_MASK != 0);
    m_ctrlKey = bool(event->key.state & GDK_CONTROL_MASK != 0);
    m_altKey = bool(event->key.state & GDK_MOD1_MASK != 0); /*alt ?*/
    m_metaKey = bool(event->key.state & GDK_MOD2_MASK != 0); /*meta ?*/

What are all those "bool" casts about? Explicit casts should not be needed.

    m_text = String(event->key.string);
    m_unmodifiedText = String(event->key.string);

And these String casts also should not be needed.

Lots of these files say Copyright 2005, 2006 in them. Did you really publish
the code you are contributing in 2005 (publish roughly meaning something like
"make available to others")? If not, then you shouldn't list 2005.

I see a mix of coding and formatting styles in this code. Most is not formatted
according to the style we use in WebCore, for example there's a lot of
8-character indenting, spaces inside parentheses, braces around single-line if
statements, return types on a separate line from function names, empty
functions are {} on one line, etc. I presume you are planning to use astyle to
fix some of the formatting issues. It would be good to fix the style to match
other WebCore code and current gudelines as much as possible before landing all
these new files, although not mandatory.


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