L. David Baron:
> 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) {
Those names look good to me.
> 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)?
For the KTables, you could define KTableValue as a class that has
implicit conversion constructors taking nsCSSKeyword and StyleBoxSizing
(and all the other new ones when they’re added).
For the casts in SetDiscrete, no, although I don’t think it’s much of a
problem. (I suppose you could do something where you have a template
wrapper class for nsCSSValue that identifies the particular enum that an
eCSSUnit_Enumerated in the wrapped nsCSSValue represents, and then have
nsRuleData::ValueForXXX return types like
nsCSSValueWrapper<StyleBoxSizing>, but it seems over the top.)
> 3. General
> ==========
>
> Do others agree this is a good idea? And are people ok with it
> being done gradually?
Yes, I think it’s great.
--
Cameron McCormack ≝ http://mcc.id.au/
_______________________________________________
dev-tech-layout mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-layout
