Auto merge of #15063 - Wafflespeanut:calc, r=heycam

Cleaning up CalcLengthOrPercentage

<!-- Please describe your changes on the following line: -->

We don't really need enum variants in `CalcLengthOrPercentage`. Given that we already have the information (whether it's `vw`, `ch`, etc.) in the struct fields, we could just store `Option<CSSFloat>` there, and modify our `ToCss` implementation a bit.

cc #15061

r? @Manishearth or anyone interested

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [x] These changes do not require tests because it's a refactor

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/15063)
<!-- Reviewable:end -->

components/style/values/computed/length.rs

@@ -10,6 +10,7 @@ use std::fmt;
 use style_traits::ToCss;
 use super::{Number, ToComputedValue, Context};
 use values::{Auto, CSSFloat, Either, None_, Normal, specified};
+use values::specified::length::{FontRelativeLength, ViewportPercentageLength};
 
 pub use cssparser::Color as CSSColor;
 pub use super::image::{EndingShape as GradientShape, Gradient, GradientKind, Image};
@@ -105,13 +106,19 @@ impl ToComputedValue for specified::CalcLengthOrPercentage {
             length += absolute;
         }
 
-        for val in &[self.vw, self.vh, self.vmin, self.vmax] {
+        for val in &[self.vw.map(ViewportPercentageLength::Vw),
+                     self.vh.map(ViewportPercentageLength::Vh),
+                     self.vmin.map(ViewportPercentageLength::Vmin),
+                     self.vmax.map(ViewportPercentageLength::Vmax)] {
             if let Some(val) = *val {
                 length += val.to_computed_value(context.viewport_size());
             }
         }
 
-        for val in &[self.ch, self.em, self.ex, self.rem] {
+        for val in &[self.ch.map(FontRelativeLength::Ch),
+                     self.em.map(FontRelativeLength::Em),
+                     self.ex.map(FontRelativeLength::Ex),
+                     self.rem.map(FontRelativeLength::Rem)] {
             if let Some(val) = *val {
                 length += val.to_computed_value(context, /* use inherited */ false);
             }
@@ -119,7 +126,7 @@ impl ToComputedValue for specified::CalcLengthOrPercentage {
 
         CalcLengthOrPercentage {
             length: length,
-            percentage: self.percentage.map(|p| p.0),
+            percentage: self.percentage,
         }
     }
 
@@ -127,7 +134,7 @@ impl ToComputedValue for specified::CalcLengthOrPercentage {
     fn from_computed_value(computed: &CalcLengthOrPercentage) -> Self {
         specified::CalcLengthOrPercentage {
             absolute: Some(computed.length),
-            percentage: computed.percentage.map(specified::Percentage),
+            percentage: computed.percentage,
             ..Default::default()
         }
     }

components/style/values/specified/length.rs

@@ -440,15 +440,15 @@ pub enum CalcUnit {
 #[allow(missing_docs)]
 pub struct CalcLengthOrPercentage {
     pub absolute: Option<Au>,
-    pub vw: Option<ViewportPercentageLength>,
+    pub vw: Option<CSSFloat>,
-    pub vh: Option<ViewportPercentageLength>,
+    pub vh: Option<CSSFloat>,
-    pub vmin: Option<ViewportPercentageLength>,
+    pub vmin: Option<CSSFloat>,
-    pub vmax: Option<ViewportPercentageLength>,
+    pub vmax: Option<CSSFloat>,
-    pub em: Option<FontRelativeLength>,
+    pub em: Option<CSSFloat>,
-    pub ex: Option<FontRelativeLength>,
+    pub ex: Option<CSSFloat>,
-    pub ch: Option<FontRelativeLength>,
+    pub ch: Option<CSSFloat>,
-    pub rem: Option<FontRelativeLength>,
+    pub rem: Option<CSSFloat>,
-    pub percentage: Option<Percentage>,
+    pub percentage: Option<CSSFloat>,
 }
 
 impl CalcLengthOrPercentage {
@@ -671,15 +671,15 @@ impl CalcLengthOrPercentage {
 
         Ok(CalcLengthOrPercentage {
             absolute: absolute.map(Au),
-            vw: vw.map(ViewportPercentageLength::Vw),
+            vw: vw,
-            vh: vh.map(ViewportPercentageLength::Vh),
+            vh: vh,
-            vmax: vmax.map(ViewportPercentageLength::Vmax),
+            vmax: vmax,
-            vmin: vmin.map(ViewportPercentageLength::Vmin),
+            vmin: vmin,
-            em: em.map(FontRelativeLength::Em),
+            em: em,
-            ex: ex.map(FontRelativeLength::Ex),
+            ex: ex,
-            ch: ch.map(FontRelativeLength::Ch),
+            ch: ch,
-            rem: rem.map(FontRelativeLength::Rem),
+            rem: rem,
-            percentage: percentage.map(Percentage),
+            percentage: percentage,
         })
     }
 
@@ -767,21 +767,26 @@ impl ToCss for CalcLengthOrPercentage {
             };
         }
 
-        macro_rules! serialize {
-            ( $( $val:ident ),* ) => {
-                {
         let mut first_value = true;
-                    $(
+        macro_rules! first_value_check {
-                        if let Some(val) = self.$val {
+            () => {
                 if !first_value {
-                                try!(write!(dest, " + "));
+                    try!(dest.write_str(" + "));
                 } else {
                     first_value = false;
                 }
+            };
+        }
+
+        macro_rules! serialize {
+            ( $( $val:ident ),* ) => {
+                $(
+                    if let Some(val) = self.$val {
+                        first_value_check!();
                         try!(val.to_css(dest));
+                        try!(dest.write_str(stringify!($val)));
                     }
                 )*
-                 }
             };
         }
 
@@ -792,11 +797,21 @@ impl ToCss for CalcLengthOrPercentage {
            try!(write!(dest, "calc("));
         }
 
-        serialize!(ch, em, ex, absolute, rem, vh, vmax, vmin, vw, percentage);
+        serialize!(ch, em, ex, rem, vh, vmax, vmin, vw);
+        if let Some(val) = self.absolute {
+            first_value_check!();
+            try!(val.to_css(dest));
+        }
+
+        if let Some(val) = self.percentage {
+            first_value_check!();
+            try!(write!(dest, "{}%", val * 100.));
+        }
 
         if count > 1 {
            try!(write!(dest, ")"));
         }
+
         Ok(())
      }
 }

tests/wpt/mozilla/tests/mozilla/calc.html

@@ -40,13 +40,13 @@ var widthTests = [
 
     // Alphabetical order
     ['calc(0ch + 0px + 0pt + 0pc + 0in + 0cm + 0mm + 0rem + 0em + 0ex + 0% + 0vw + 0vh + 0vmin + 0vmax)',
-     'calc(0ch + 0em + 0ex + 0px + 0rem + 0vh + 0vmax + 0vmin + 0vw + 0%)',
+     'calc(0ch + 0em + 0ex + 0rem + 0vh + 0vmax + 0vmin + 0vw + 0px + 0%)',
      '0px'],
 
     // Simplification
     ['calc((2 - 1) * 10px)', '10px', '10px'],
     ['calc(((3 - 1) * (8 + 4)) * 10px)', '240px', '240px'],
-    ['calc(5 * (20px / 2 + 7 * (3em + 12px/4 + (8 - 2) * 2rem)))', 'calc(105em + 155px + 420rem)', '8555px'],
+    ['calc(5 * (20px / 2 + 7 * (3em + 12px/4 + (8 - 2) * 2rem)))', 'calc(105em + 420rem + 155px)', '8555px'],
 
 ];