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

[bugzilla-daemon at opendarwin.org]

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





------- Comment #53 from mike.emmel at gmail.com  2006-06-30 12:16 PDT -------
(In reply to comment #49)
> (From update of attachment 9064 [edit])
> There is still a lot here that's not formatted right. Extra spaces in
> parentheses, missing spaces after if keywords, commas, and around operators,
> const after type in some files. It doesn't seem too challenging to fix these.
> If we don't fix it now, I'm not sure when we would.
> 


Does xcode format correctly ?
I'm happy to hack Astyle to get it close to the format rules if
your willing to work with me on it. I prefer command line formatters.
My point is it's a lot better to find a automatic formatter thats close then
document what you have to look for by hand then it is to format code by hand.
Right now I'm having to hand format this code which is not a productive use of
your time nor mine. I'm willing to load the code into any ide or emacs or run
any formatter program on it to get the format to match the rules.
But we should have a way to auto format the code that at least gets close.
I fixed what I could find but I'm a real believer in autoformatting and as I
said I'd be happy to figure out what the process should be to remove or lessen
the need for hand formatting.

In any case I agree these need to be fixed and I tried to fix as much as 
I could find.




> I'm also wondering about file names with "Gdk" in them -- shouldn't it be all
> capitals GDK?
> 

No the gdk sources use GdkWindow etc for classes. This is right.

> We don't normally check in #if 0'd code and commented out code, and there's a
> lot of both here.
> 
> Macro names need to be all capitals.
> 
> Comments like this:
> 
> +    //XXX create a new one here !
> 
Fixed
> aren't really useful unless other people on the project know what "XXX" means
> what a "new one" is.
> 
> We try to keep all the stubs in a single file, but 
> I'm mystified by other loose ends like the stubs and loadResourceIntoArray
> function which are defined in FrameGdk.cpp -- clearly seems the wrong place for
> these things.
> 
Yes and I expanded the comment the problem is the function was added
and probably should be a entry point into a resource loader but there
is no code for loading none code resources and I have no idea what the  plans
are are we going to have a platform independent resource loader ?
It placement is to invoke questions plus the main external class to override
right now is GdkFrame so this is where you could plugin your own resource
loader
not that its the best place.


> Now that the constants in KeyboardCodes.h are plain old namespaced const, they
> should no longer be all capitals -- that's for macros only.

Not a problem but lets change this file one time its a lot of hand editing.
It should not be here either since its platform independent.
My suggestion is leave it alone for now lets review it later for moving up
into platform and fix it once. I'm not sure how you want it named do you 
want  VK_TAB to go to
VK_Tab
KeyCodeTab
TabKC
???
We don't have to keep the VK_
I'm fine with changing it but lets do it once and move it up into platform.



> 
> +                                      (GSourceFunc)timeout_cb,
> 
> We don't do typecasts on function types because that often leads to trouble.
> Instead we use function types that exactly match and do typecasting on the
> types inside the functions -- a lot safer -- I can go into more detail about
> this if you like. It's something I really learned the importance of while
> working on Nautilus for the Gnome project.
> 
Fixed I agree

> RenderThemeGdk.cpp has comments in it about Windows Theme API that are not
> correct or appropriate.
> 
>  #include "kjs_html.lut.h"
> +#include <math.h>
> 
> This include is in the wrong place; should be up with the other includes, not
> after the "lut.h" include.
> 
Fixed

> On the other hand, it's possible that I should ignore these issues and
> rubber-stamp this so we can get the GDK port into the tree, regardless of the
> many small problems in the code.
> 
> But I'm not entirely comfortable with that. I'd like to see as many of these
> issues as possible fixed. I'd feel a lot better about taking this into the tree
> if it didn't have all the minor problems I mention above.
> 


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