[Webkit-unassigned] [Bug 15914] [GTK] Implement Unicode functionality using GLib

[bugzilla-daemon at webkit.org]

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


eric at webkit.org changed:

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




------- Comment #34 from eric at webkit.org  2008-09-23 04:34 PDT -------
(From update of attachment 23263)
There are a bunch of webkit-style violations in this patch, which make it
difficult to r+.

It concerns me that you'd need to copy all those macros from the ICU headers. 
Does GLIB not provide any equivalents?

Things like where * and & go, relative to the variable name.
Single line if/else statements don't use {}
Use of c-style casts (Foo*)foo instead of static_cast<Foo*>(foo)
Each variable gets its own declaration line, no int foo, bar;
4-space indents
0 instead of NULL when referring to pointers in c++ code
true and false instead of TRUE and FALSE when dealing with c++ "bool" types
Header #define names should be identical to the header, like #define
UnicodeGLib_h

Anything copied from Glib should really be in its own header file, IMO. 
Separating things out into their own file makes it easier to update them if
they ever change upstream, or for platforms which have the necessary headers to
not use our local copies of that data.

Probably the same goes of the ICU macros. UnicodeMacrosFromICU.h would be one
possible name. :)

WebKit source code files are generally UT8, I'm not sure what encoding you're
editing with:
+ * Copyright (C) 2008 Jürg Billeter <j at bitron.ch>


There is a lot of code in UnicodeGLib.h, much of that could be moved into a
corresponding .cpp file.

:(  Even though this code touches Gtk, it would be best if we could avoid
manual-menory management.  Manual memory management is *evil* (IMO) and gets
you into all sorts of trouble.  I think there was a GOwnPtr landed recently,
no?  That would know how to call g_free on its contents when it went out of
scope?  If so, one could use that instead of all the manual g_free calls.

Thanks for the patch, but I think we need another round of cleanup to this one.


-- 
Configure bugmail: https://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.