[Webkit-unassigned] [Bug 34571] [Haiku] Implementation of browser shell

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 4 22:41:35 PST 2010


https://bugs.webkit.org/show_bug.cgi?id=34571





--- Comment #6 from Stephan Aßmus <superstippi at gmx.de>  2010-02-04 22:41:33 PST ---
(In reply to comment #5)
> (From update of attachment 48128 [details])
> 
> > +enum {
> > +    HANDLE_SHUTDOWN                        = 'sdwn',
> > +
> > +    HANDLE_LOAD_URL                        = 'lurl',
> > +    HANDLE_GO_BACK                        = 'back',
> > +    HANDLE_GO_FORWARD                    = 'fwrd',
> > +
> > +    HANDLE_DRAW                            = 'draw',
> > +
> > +    HANDLE_FRAME_RESIZED                = 'rszd'
> > +};
> 
> This is incorrect, character literals should be a single character, this
> triggers warnings in GCC and likely does not do what you are expecting.

These are not character literals, but a 32 bit integer enumeration (on 32 bit
host). I can absolutely assure you that this code is correct and does what it
is supposed to. I am also not getting GCC warnings. Code like the above is used
on BeOS since years to define BMessage "what" constants used in event handler
switch blocks. But the code could be rewritten as such:

const uint32 kHandleShutDown = 'sdwn';
const uint32 kHandleLoadURL = 'lurl';
const uint32 kHandleGoBack = 'back';
[...]

That would be equivalent. Would you prefer that?

> Overall i'd also prefer this patch was split into multiple pieces so that it's
> easier to review.

Ok. In which way should I separate the patches? Individual files for each
class? Or multiple files grouped by function?

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


More information about the webkit-unassigned mailing list