Auto merge of #19938 - servo:single-keyword, r=emilio

Simplify single_keyword

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19938)
<!-- Reviewable:end -->

components/style/properties/helpers.mako.rs

@@ -490,33 +490,6 @@
     </%call>
 </%def>
 
-<%def name="single_keyword(name, values, vector=False, **kwargs)">
-    <%call expr="single_keyword_computed(name, values, vector, **kwargs)">
-        // FIXME(emilio): WTF is this even trying to do? Those are no-ops,
-        // should be derived instead!
-        impl ToComputedValue for SpecifiedValue {
-            type ComputedValue = computed_value::T;
-
-            #[inline]
-            fn to_computed_value(&self, _context: &Context) -> computed_value::T {
-                match *self {
-                    % for value in data.longhands_by_name[name].keyword.values_for(product):
-                        SpecifiedValue::${to_camel_case(value)} => computed_value::T::${to_camel_case(value)},
-                    % endfor
-                }
-            }
-            #[inline]
-            fn from_computed_value(computed: &computed_value::T) -> Self {
-                match *computed {
-                    % for value in data.longhands_by_name[name].keyword.values_for(product):
-                        computed_value::T::${to_camel_case(value)} => SpecifiedValue::${to_camel_case(value)},
-                    % endfor
-                }
-            }
-        }
-    </%call>
-</%def>
-
 <%def name="gecko_keyword_conversion(keyword, values=None, type='SpecifiedValue', cast_to=None)">
     <%
         if not values:
@@ -584,7 +557,7 @@
     }
 </%def>
 
-<%def name="single_keyword_computed(name, values, vector=False,
+<%def name="single_keyword(name, values, vector=False,
             extra_specified=None, needs_conversion=False, **kwargs)">
     <%
         keyword_kwargs = {a: kwargs.pop(a, None) for a in [
@@ -596,33 +569,39 @@
     %>
 
     <%def name="inner_body(keyword, extra_specified=None, needs_conversion=False)">
-        % if extra_specified or keyword.aliases_for(product):
+        <%def name="variants(variants, include_aliases)">
+            % for variant in variants:
+            % if include_aliases:
+            <%
+                aliases = []
+                for alias, v in keyword.aliases_for(product).iteritems():
+                    if variant == v:
+                        aliases.append(alias)
+            %>
+            % if aliases:
+            #[css(aliases = "${','.join(aliases)}")]
+            % endif
+            % endif
+            ${to_camel_case(variant)},
+            % endfor
+        </%def>
+        % if extra_specified:
             #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
             #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)]
             pub enum SpecifiedValue {
-                % for value in keyword.values_for(product) + (extra_specified or "").split():
-                <%
-                    aliases = []
-                    for alias, v in keyword.aliases_for(product).iteritems():
-                        if value == v:
-                            aliases.append(alias)
-                %>
-                % if aliases:
-                #[css(aliases = "${','.join(aliases)}")]
-                % endif
-                ${to_camel_case(value)},
-                % endfor
+                ${variants(keyword.values_for(product) + extra_specified.split(), bool(extra_specified))}
             }
         % else:
             pub use self::computed_value::T as SpecifiedValue;
         % endif
         pub mod computed_value {
             #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
-            #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)]
+            #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToCss)]
+            % if not extra_specified:
+            #[derive(Parse, ToComputedValue)]
+            % endif
             pub enum T {
-            % for value in data.longhands_by_name[name].keyword.values_for(product):
-                ${to_camel_case(value)},
-            % endfor
+                ${variants(data.longhands_by_name[name].keyword.values_for(product), not extra_specified)}
             }
         }
         #[inline]
@@ -652,13 +631,17 @@
     % if vector:
         <%call expr="vector_longhand(name, keyword=Keyword(name, values, **keyword_kwargs), **kwargs)">
             ${inner_body(Keyword(name, values, **keyword_kwargs))}
+            % if caller:
             ${caller.body()}
+            % endif
         </%call>
     % else:
         <%call expr="longhand(name, keyword=Keyword(name, values, **keyword_kwargs), **kwargs)">
             ${inner_body(Keyword(name, values, **keyword_kwargs),
                          extra_specified=extra_specified, needs_conversion=needs_conversion)}
+            % if caller:
             ${caller.body()}
+            % endif
         </%call>
     % endif
 </%def>

components/style/properties/longhand/box.mako.rs

@@ -54,17 +54,19 @@ ${helpers.single_keyword("position", "static absolute relative fixed sticky",
                          flags="CREATES_STACKING_CONTEXT ABSPOS_CB",
                          spec="https://drafts.csswg.org/css-position/#position-property")}
 
-<%helpers:single_keyword_computed name="float"
-                                  values="none left right"
-                                  // https://drafts.csswg.org/css-logical-props/#float-clear
-                                  extra_specified="inline-start inline-end"
-                                  needs_conversion="True"
-                                  animation_value_type="discrete"
-                                  gecko_enum_prefix="StyleFloat"
-                                  gecko_inexhaustive="True"
-                                  gecko_ffi_name="mFloat"
-                                  flags="APPLIES_TO_FIRST_LETTER"
-                                  spec="https://drafts.csswg.org/css-box/#propdef-float">
+<%helpers:single_keyword
+    name="float"
+    values="none left right"
+    // https://drafts.csswg.org/css-logical-props/#float-clear
+    extra_specified="inline-start inline-end"
+    needs_conversion="True"
+    animation_value_type="discrete"
+    gecko_enum_prefix="StyleFloat"
+    gecko_inexhaustive="True"
+    gecko_ffi_name="mFloat"
+    flags="APPLIES_TO_FIRST_LETTER"
+    spec="https://drafts.csswg.org/css-box/#propdef-float"
+>
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
@@ -105,18 +107,20 @@ ${helpers.single_keyword("position", "static absolute relative fixed sticky",
             }
         }
     }
-</%helpers:single_keyword_computed>
+</%helpers:single_keyword>
 
-<%helpers:single_keyword_computed name="clear"
-                                  values="none left right both"
-                                  // https://drafts.csswg.org/css-logical-props/#float-clear
-                                  extra_specified="inline-start inline-end"
-                                  needs_conversion="True"
-                                  gecko_inexhaustive="True"
-                                  animation_value_type="discrete"
-                                  gecko_enum_prefix="StyleClear"
-                                  gecko_ffi_name="mBreakType"
-                                  spec="https://www.w3.org/TR/CSS2/visuren.html#flow-control">
+<%helpers:single_keyword
+    name="clear"
+    values="none left right both"
+    // https://drafts.csswg.org/css-logical-props/#float-clear
+    extra_specified="inline-start inline-end"
+    needs_conversion="True"
+    gecko_inexhaustive="True"
+    animation_value_type="discrete"
+    gecko_enum_prefix="StyleClear"
+    gecko_ffi_name="mBreakType"
+    spec="https://drafts.csswg.org/css-box/#propdef-clear"
+>
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
@@ -157,7 +161,7 @@ ${helpers.single_keyword("position", "static absolute relative fixed sticky",
             }
         }
     }
-</%helpers:single_keyword_computed>
+</%helpers:single_keyword>
 
 ${helpers.predefined_type(
     "vertical-align",

components/style/properties/longhand/inherited_text.mako.rs

@@ -60,46 +60,43 @@ ${helpers.single_keyword("word-break",
                          spec="https://drafts.csswg.org/css-text/#propdef-word-break")}
 
 // TODO(pcwalton): Support `text-justify: distribute`.
-<%helpers:single_keyword_computed name="text-justify"
-                                  values="auto none inter-word"
-                                  extra_gecko_values="inter-character"
-                                  extra_specified="${'distribute' if product == 'gecko' else ''}"
-                                  gecko_enum_prefix="StyleTextJustify"
-                                  animation_value_type="discrete"
-                                  gecko_pref="layout.css.text-justify.enabled"
-                                  flags="APPLIES_TO_PLACEHOLDER",
-                                  spec="https://drafts.csswg.org/css-text/#propdef-text-justify">
-
+<%helpers:single_keyword
+    name="text-justify"
+    values="auto none inter-word"
+    extra_gecko_values="inter-character"
+    extra_specified="${'distribute' if product == 'gecko' else ''}"
+    gecko_enum_prefix="StyleTextJustify"
+    animation_value_type="discrete"
+    gecko_pref="layout.css.text-justify.enabled"
+    flags="APPLIES_TO_PLACEHOLDER",
+    spec="https://drafts.csswg.org/css-text/#propdef-text-justify"
+>
+    % if product == 'gecko':
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
         #[inline]
         fn to_computed_value(&self, _: &Context) -> computed_value::T {
             match *self {
-                % for value in "Auto None InterWord".split():
-                    SpecifiedValue::${value} => computed_value::T::${value},
+                % for value in "Auto None InterCharacter InterWord".split():
+                SpecifiedValue::${value} => computed_value::T::${value},
                 % endfor
-                % if product == "gecko":
-                    SpecifiedValue::InterCharacter => computed_value::T::InterCharacter,
-                    // https://drafts.csswg.org/css-text-3/#valdef-text-justify-distribute
-                    SpecifiedValue::Distribute => computed_value::T::InterCharacter,
-                % endif
+                // https://drafts.csswg.org/css-text-3/#valdef-text-justify-distribute
+                SpecifiedValue::Distribute => computed_value::T::InterCharacter,
             }
         }
 
         #[inline]
         fn from_computed_value(computed: &computed_value::T) -> SpecifiedValue {
             match *computed {
-                % for value in "Auto None InterWord".split():
-                    computed_value::T::${value} => SpecifiedValue::${value},
+                % for value in "Auto None InterCharacter InterWord".split():
+                computed_value::T::${value} => SpecifiedValue::${value},
                 % endfor
-                % if product == "gecko":
-                    computed_value::T::InterCharacter => SpecifiedValue::InterCharacter,
-                % endif
             }
         }
     }
-</%helpers:single_keyword_computed>
+    % endif
+</%helpers:single_keyword>
 
 ${helpers.single_keyword("text-align-last",
                          "auto start end left right center justify",
@@ -132,17 +129,17 @@ ${helpers.predefined_type("word-spacing",
                           flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER",
                           spec="https://drafts.csswg.org/css-text/#propdef-word-spacing")}
 
-<%helpers:single_keyword_computed name="white-space"
-                                  values="normal pre nowrap pre-wrap pre-line"
-                                  extra_gecko_values="-moz-pre-space"
-                                  gecko_enum_prefix="StyleWhiteSpace"
-                                  needs_conversion="True"
-                                  animation_value_type="discrete"
-                                  // Only allowed for UA sheets, which set it
-                                  // !important.
-                                  flags="APPLIES_TO_PLACEHOLDER"
-                                  spec="https://drafts.csswg.org/css-text/#propdef-white-space">
-    trivial_to_computed_value!(SpecifiedValue);
+<%helpers:single_keyword
+    name="white-space"
+    values="normal pre nowrap pre-wrap pre-line"
+    extra_gecko_values="-moz-pre-space"
+    gecko_enum_prefix="StyleWhiteSpace"
+    needs_conversion="True"
+    animation_value_type="discrete"
+    // Only allowed for UA sheets, which set it !important.
+    flags="APPLIES_TO_PLACEHOLDER"
+    spec="https://drafts.csswg.org/css-text/#propdef-white-space"
+>
     % if product != "gecko":
     impl SpecifiedValue {
         pub fn allow_wrap(&self) -> bool {
@@ -176,7 +173,7 @@ ${helpers.predefined_type("word-spacing",
         }
     }
     % endif
-</%helpers:single_keyword_computed>
+</%helpers:single_keyword>
 
 ${helpers.predefined_type(
     "text-shadow",