[SVG] Start moving special casing of SVG out of the bindings - SVGAngle
Created attachment 294104 [details] Patch
Attachment 294104 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGAngleValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:40: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:41: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 5 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 294104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294104&action=review > Source/WebCore/svg/SVGAngle.h:40 > + static Ref<SVGAngle> create(SVGAnimatedProperty* animatedProperty, SVGPropertyRole role, SVGAngleValue& value) Does this first argument need to be a pointer rather than a reference? > Source/WebCore/svg/SVGAngle.h:140 > + SVGAngle(SVGAngleValue initialValue) explicit? > Source/WebCore/svg/SVGAngleValue.h:34 > +class SVGAngleValue { > + WTF_MAKE_FAST_ALLOCATED; Why WTF_MAKE_FAST_ALLOCATED? Who allocates these on the heap? > Source/WebCore/svg/SVGAngleValue.h:42 > + enum Type { > + SVG_ANGLETYPE_UNKNOWN = 0, > + SVG_ANGLETYPE_UNSPECIFIED = 1, > + SVG_ANGLETYPE_DEG = 2, > + SVG_ANGLETYPE_RAD = 3, > + SVG_ANGLETYPE_GRAD = 4 > + }; Annoying to repeat this enum as SVGAngleValue::Type and as SVGAngle::SVGAngleType. Would be bad if the two ever got out of sync. Any ideas on how to avoid that? > Source/WebCore/svg/SVGPathSegList.h:31 > class SVGElement; > +template<typename T> class SVGPropertyTearOff; Blank line here so the templates are in a separate paragraph from the classes? > Source/WebCore/svg/SVGStringList.h:30 > class SVGElement; > +template<typename T> class SVGPropertyTearOff; Ditto.
(In reply to comment #3) > Comment on attachment 294104 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=294104&action=review > > > Source/WebCore/svg/SVGAngle.h:40 > > + static Ref<SVGAngle> create(SVGAnimatedProperty* animatedProperty, SVGPropertyRole role, SVGAngleValue& value) > > Does this first argument need to be a pointer rather than a reference? Good question. I am duplicating the interface of SVGPropertyTearOff here, so I didn't want to change anything like that. I'll find out before landing. > > > Source/WebCore/svg/SVGAngle.h:140 > > + SVGAngle(SVGAngleValue initialValue) > > explicit? Will change. > > > Source/WebCore/svg/SVGAngleValue.h:34 > > +class SVGAngleValue { > > + WTF_MAKE_FAST_ALLOCATED; > > Why WTF_MAKE_FAST_ALLOCATED? Who allocates these on the heap? SVGPropertyTearOff (and therefore SVGAngle) allocates these. See the SVGPropertyTearOff constructor that takes a const PropertyType*. > > > Source/WebCore/svg/SVGAngleValue.h:42 > > + enum Type { > > + SVG_ANGLETYPE_UNKNOWN = 0, > > + SVG_ANGLETYPE_UNSPECIFIED = 1, > > + SVG_ANGLETYPE_DEG = 2, > > + SVG_ANGLETYPE_RAD = 3, > > + SVG_ANGLETYPE_GRAD = 4 > > + }; > > Annoying to repeat this enum as SVGAngleValue::Type and as > SVGAngle::SVGAngleType. Would be bad if the two ever got out of sync. Any > ideas on how to avoid that? I was really sad about this. I tried a few things, like trying to use a using statement to re-export the enum in another place. I think I will probably need to update the bindings to be able to look in an alternate place for these. > > > Source/WebCore/svg/SVGPathSegList.h:31 > > class SVGElement; > > +template<typename T> class SVGPropertyTearOff; > > Blank line here so the templates are in a separate paragraph from the > classes? Ok. > > > Source/WebCore/svg/SVGStringList.h:30 > > class SVGElement; > > +template<typename T> class SVGPropertyTearOff; > > Ditto. Ok.
Created attachment 294271 [details] Patch
Attachment 294271 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGAngleValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGAngleValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 5 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 294271 [details] Patch Clearing flags on attachment: 294271 Committed r208480: <http://trac.webkit.org/changeset/208480>
All reviewed patches have been landed. Closing bug.