Jesse suggested using enum classes for the values of enumerated CSS properties, and I tend to agree that this is a good idea. It gives both stricter typechecking than the current uint8_t (etc.) values, and better warnings for switch() statements omitting values.
I decided to try this out for box-sizing to see what it would look like. Patches are linked from: https://bugzilla.mozilla.org/show_bug.cgi?id=1223653#c1 Interestingly, the typechecking actually caught a real bug, where we were passing an nscoord (size inside the box-sizing) to a function that expected a uint8_t (enumerated value of box-sizing property), because we actually intended to call a different function. But I'm mainly interested in feedback on: 1. Naming ========= Do the names I chose seem reasonable? I replaced: -#define NS_STYLE_BOX_SIZING_CONTENT 0 -#define NS_STYLE_BOX_SIZING_PADDING 1 -#define NS_STYLE_BOX_SIZING_BORDER 2 with: +namespace mozilla { + +enum class StyleBoxSizing : uint8_t { + Content, + Padding, + Border +}; + +} // namespace mozilla so that most caller changes look like: - case NS_STYLE_BOX_SIZING_BORDER: + case StyleBoxSizing::Border: or like: if (parent && - parent->StylePosition()->mBoxSizing != NS_STYLE_BOX_SIZING_BORDER) { + parent->StylePosition()->mBoxSizing != StyleBoxSizing::Border) { 2. Casts ======== Does anybody see a good way to eliminate the casts I needed to use in the main patch (described in the patch header)? 3. General ========== Do others agree this is a good idea? And are people ok with it being done gradually? -David -- 𝄞 L. David Baron http://dbaron.org/ 𝄂 𝄢 Mozilla https://www.mozilla.org/ 𝄂 Before I built a wall I'd ask to know What I was walling in or walling out, And to whom I was like to give offense. - Robert Frost, Mending Wall (1914)
signature.asc
Description: Digital signature
_______________________________________________ dev-tech-layout mailing list [email protected] https://lists.mozilla.org/listinfo/dev-tech-layout
