From 39e29f557edc6a06f0fd983bd8c456799aba844d Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 21 Jun 2017 10:15:31 +0200 Subject: [PATCH] Simplify machinery to serialise optional parts of CSS values We simply implement ToCss for Option, printing nothing if the value is None, and we then use SequenceWriter to skip writing of separators around empty parts. --- .../style/properties/longhand/border.mako.rs | 14 +- .../longhand/inherited_table.mako.rs | 17 +-- .../style/values/generics/basic_shape.rs | 18 +-- components/style/values/generics/image.rs | 15 +- components/style/values/specified/effects.rs | 28 +--- components/style/values/specified/position.rs | 23 +-- components/style_derive/to_css.rs | 24 ++- components/style_traits/values.rs | 144 ++++++++++++++++-- 8 files changed, 152 insertions(+), 131 deletions(-) diff --git a/components/style/properties/longhand/border.mako.rs b/components/style/properties/longhand/border.mako.rs index d337c619607..d984a2d80d5 100644 --- a/components/style/properties/longhand/border.mako.rs +++ b/components/style/properties/longhand/border.mako.rs @@ -214,7 +214,6 @@ ${helpers.predefined_type("border-image-outset", "LengthOrNumberRect", <%helpers:longhand name="border-image-repeat" animation_value_type="discrete" spec="https://drafts.csswg.org/css-backgrounds/#border-image-repeat"> - use std::fmt; use style_traits::ToCss; no_viewport_percentage!(SpecifiedValue); @@ -227,8 +226,8 @@ ${helpers.predefined_type("border-image-outset", "LengthOrNumberRect", pub struct T(pub RepeatKeyword, pub RepeatKeyword); } - #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] + #[derive(Debug, Clone, PartialEq, ToCss)] pub struct SpecifiedValue(pub RepeatKeyword, pub Option); @@ -238,17 +237,6 @@ ${helpers.predefined_type("border-image-outset", "LengthOrNumberRect", "round" => Round, "space" => Space); - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - self.0.to_css(dest)?; - if let Some(second) = self.1 { - dest.write_str(" ")?; - second.to_css(dest)?; - } - Ok(()) - } - } - #[inline] pub fn get_initial_value() -> computed_value::T { computed_value::T(RepeatKeyword::Stretch, RepeatKeyword::Stretch) diff --git a/components/style/properties/longhand/inherited_table.mako.rs b/components/style/properties/longhand/inherited_table.mako.rs index 14d99df73d3..44ddc1881c9 100644 --- a/components/style/properties/longhand/inherited_table.mako.rs +++ b/components/style/properties/longhand/inherited_table.mako.rs @@ -23,8 +23,6 @@ ${helpers.single_keyword("caption-side", "top bottom", <%helpers:longhand name="border-spacing" animation_value_type="ComputedValue" boxed="True" spec="https://drafts.csswg.org/css-tables/#propdef-border-spacing"> use app_units::Au; - use std::fmt; - use style_traits::ToCss; use values::specified::{AllowQuirks, Length}; pub mod computed_value { @@ -64,8 +62,8 @@ ${helpers.single_keyword("caption-side", "top bottom", } } - #[derive(Clone, Debug, HasViewportPercentage, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] + #[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToCss)] pub struct SpecifiedValue { pub horizontal: Length, pub vertical: Option, @@ -79,19 +77,6 @@ ${helpers.single_keyword("caption-side", "top bottom", } } - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result - where W: fmt::Write, - { - self.horizontal.to_css(dest)?; - if let Some(vertical) = self.vertical.as_ref() { - dest.write_str(" ")?; - vertical.to_css(dest)?; - } - Ok(()) - } - } - impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index 1d54a90dbca..e7cfdbd25fe 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -43,7 +43,7 @@ add_impls_for_keyword_enum!(ShapeBox); /// A shape source, for some reference box. #[allow(missing_docs)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Clone, Debug, PartialEq, ToComputedValue)] +#[derive(Clone, Debug, PartialEq, ToComputedValue, ToCss)] pub enum ShapeSource { Url(SpecifiedUrl), Shape(BasicShape, Option), @@ -126,22 +126,6 @@ impl HasViewportPercentage for ShapeSource { fn has_viewport_percentage(&self) -> bool { false } } -impl ToCss for ShapeSource { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match *self { - ShapeSource::Url(ref url) => url.to_css(dest), - ShapeSource::Shape(ref shape, Some(ref ref_box)) => { - shape.to_css(dest)?; - dest.write_str(" ")?; - ref_box.to_css(dest) - }, - ShapeSource::Shape(ref shape, None) => shape.to_css(dest), - ShapeSource::Box(ref val) => val.to_css(dest), - ShapeSource::None => dest.write_str("none"), - } - } -} - impl ToCss for InsetRect where L: ToCss + PartialEq { diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index 20094e75935..36d30028787 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -123,7 +123,7 @@ pub enum GradientItem { /// A color stop. /// https://drafts.csswg.org/css-images/#typedef-color-stop-list -#[derive(Clone, Copy, HasViewportPercentage, PartialEq, ToComputedValue)] +#[derive(Clone, Copy, HasViewportPercentage, PartialEq, ToComputedValue, ToCss)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct ColorStop { /// The color of this stop. @@ -321,19 +321,6 @@ impl fmt::Debug for ColorStop } } -impl ToCss for ColorStop - where C: ToCss, L: ToCss, -{ - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - self.color.to_css(dest)?; - if let Some(ref position) = self.position { - dest.write_str(" ")?; - position.to_css(dest)?; - } - Ok(()) - } -} - impl ToCss for ImageRect where C: ToCss, { diff --git a/components/style/values/specified/effects.rs b/components/style/values/specified/effects.rs index 9be883042f1..7987ddc8169 100644 --- a/components/style/values/specified/effects.rs +++ b/components/style/values/specified/effects.rs @@ -6,13 +6,9 @@ use cssparser::{BasicParseError, Parser, Token}; use parser::{Parse, ParserContext}; -#[cfg(feature = "gecko")] -use std::fmt; use style_traits::ParseError; #[cfg(not(feature = "gecko"))] use style_traits::StyleParseError; -#[cfg(feature = "gecko")] -use style_traits::ToCss; use values::computed::{Context, Number as ComputedNumber, ToComputedValue}; use values::computed::effects::DropShadow as ComputedDropShadow; use values::generics::effects::Filter as GenericFilter; @@ -55,7 +51,7 @@ pub enum DropShadow {} /// Contrary to the canonical order from the spec, the color is serialised /// first, like in Gecko's computed values and in all Webkit's values. #[cfg(feature = "gecko")] -#[derive(Clone, Debug, HasViewportPercentage, PartialEq)] +#[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToCss)] pub struct DropShadow { /// Color. pub color: Option, @@ -220,25 +216,3 @@ impl ToComputedValue for DropShadow { } } } - -#[cfg(feature = "gecko")] -impl ToCss for DropShadow { - #[inline] - fn to_css(&self, dest: &mut W) -> fmt::Result - where - W: fmt::Write, - { - if let Some(ref color) = self.color { - color.to_css(dest)?; - dest.write_str(" ")?; - } - self.horizontal.to_css(dest)?; - dest.write_str(" ")?; - self.vertical.to_css(dest)?; - if let Some(ref blur) = self.blur { - dest.write_str(" ")?; - blur.to_css(dest)?; - } - Ok(()) - } -} diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index ccbc861b3c1..4353a0fafa5 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -27,7 +27,7 @@ pub type VerticalPosition = PositionComponent; /// The specified value of a component of a CSS ``. #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, ToCss)] pub enum PositionComponent { /// `center` Center, @@ -197,27 +197,6 @@ impl PositionComponent { } } -impl ToCss for PositionComponent { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match *self { - PositionComponent::Center => { - dest.write_str("center") - }, - PositionComponent::Length(ref lop) => { - lop.to_css(dest) - }, - PositionComponent::Side(ref keyword, ref lop) => { - keyword.to_css(dest)?; - if let Some(ref lop) = *lop { - dest.write_str(" ")?; - lop.to_css(dest)?; - } - Ok(()) - }, - } - } -} - impl ToComputedValue for PositionComponent { type ComputedValue = ComputedLengthOrPercentage; diff --git a/components/style_derive/to_css.rs b/components/style_derive/to_css.rs index 6f705ddd91b..550da7d38e1 100644 --- a/components/style_derive/to_css.rs +++ b/components/style_derive/to_css.rs @@ -17,24 +17,22 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens { let style = synstructure::BindStyle::Ref.into(); let match_body = synstructure::each_variant(&input, &style, |bindings, variant| { let mut identifier = to_css_identifier(variant.ident.as_ref()); - let mut expr = if let Some((first, rest)) = bindings.split_first() { - if has_free_params(&first.field.ty, &input.generics.ty_params) { - where_clause.predicates.push(where_predicate(first.field.ty.clone())); - } - let mut expr = quote! { - ::style_traits::ToCss::to_css(#first, dest) - }; - for binding in rest { + let mut expr = if !bindings.is_empty() { + let mut expr = quote! {}; + for binding in bindings { if has_free_params(&binding.field.ty, &input.generics.ty_params) { where_clause.predicates.push(where_predicate(binding.field.ty.clone())); } expr = quote! { - #expr?; - ::std::fmt::Write::write_str(dest, " ")?; - ::style_traits::ToCss::to_css(#binding, dest) + #expr + writer.item(#binding)?; }; } - expr + quote! {{ + let mut writer = ::style_traits::values::SequenceWriter::new(&mut *dest, " "); + #expr + Ok(()) + }} } else { quote! { ::std::fmt::Write::write_str(dest, #identifier) @@ -80,7 +78,7 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens { quote! { impl #impl_generics ::style_traits::ToCss for #name #ty_generics #where_clause { - #[allow(unused_variables, unused_imports)] + #[allow(unused_variables)] #[inline] fn to_css(&self, dest: &mut W) -> ::std::fmt::Result where diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index 8f17edf4caf..5f8760ea58b 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -6,7 +6,7 @@ use app_units::Au; use cssparser::{UnicodeRange, serialize_string}; -use std::fmt; +use std::fmt::{self, Write}; /// Serialises a value according to its CSS representation. /// @@ -22,7 +22,7 @@ use std::fmt; /// of their fields. pub trait ToCss { /// Serialize `self` in CSS syntax, writing to `dest`. - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write; + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write; /// Serialize `self` in CSS syntax and return a string. /// @@ -36,25 +36,151 @@ pub trait ToCss { } impl<'a, T> ToCss for &'a T where T: ToCss + ?Sized { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { (*self).to_css(dest) } } impl ToCss for str { #[inline] - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { serialize_string(self, dest) } } impl ToCss for String { #[inline] - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { serialize_string(self, dest) } } +impl ToCss for Option +where + T: ToCss, +{ + #[inline] + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { + self.as_ref().map_or(Ok(()), |value| value.to_css(dest)) + } +} + +/// Convenience wrapper to serialise CSS values separated by a given string. +pub struct SequenceWriter<'a, W> { + writer: TrackedWriter, + separator: &'a str, +} + +impl<'a, W> SequenceWriter<'a, W> +where + W: Write, +{ + /// Create a new sequence writer. + #[inline] + pub fn new(writer: W, separator: &'a str) -> Self { + SequenceWriter { + writer: TrackedWriter::new(writer), + separator: separator, + } + } + + /// Serialises a CSS value, writing any separator as necessary. + /// + /// The separator is never written before any `item` produces any output, + /// and is written in subsequent calls only if the `item` produces some + /// output on its own again. This lets us handle `Option` fields by + /// just not printing anything on `None`. + #[inline] + pub fn item(&mut self, item: &T) -> fmt::Result + where + T: ToCss, + { + if self.writer.has_written { + item.to_css(&mut PrefixedWriter::new(&mut self.writer, self.separator)) + } else { + item.to_css(&mut self.writer) + } + } +} + +struct TrackedWriter { + writer: W, + has_written: bool, +} + +impl TrackedWriter +where + W: Write, +{ + #[inline] + fn new(writer: W) -> Self { + TrackedWriter { + writer: writer, + has_written: false, + } + } +} + +impl Write for TrackedWriter +where + W: Write, +{ + #[inline] + fn write_str(&mut self, s: &str) -> fmt::Result { + if !s.is_empty() { + self.has_written = true; + } + self.writer.write_str(s) + } + + #[inline] + fn write_char(&mut self, c: char) -> fmt::Result { + self.has_written = true; + self.writer.write_char(c) + } +} + +struct PrefixedWriter<'a, W> { + writer: W, + prefix: Option<&'a str>, +} + +impl<'a, W> PrefixedWriter<'a, W> +where + W: Write, +{ + #[inline] + fn new(writer: W, prefix: &'a str) -> Self { + PrefixedWriter { + writer: writer, + prefix: Some(prefix), + } + } +} + +impl<'a, W> Write for PrefixedWriter<'a, W> +where + W: Write, +{ + #[inline] + fn write_str(&mut self, s: &str) -> fmt::Result { + if !s.is_empty() { + if let Some(prefix) = self.prefix.take() { + self.writer.write_str(prefix)?; + } + } + self.writer.write_str(s) + } + + #[inline] + fn write_char(&mut self, c: char) -> fmt::Result { + if let Some(prefix) = self.prefix.take() { + self.writer.write_str(prefix)?; + } + self.writer.write_char(c) + } +} + /// Type used as the associated type in the `OneOrMoreSeparated` trait on a /// type to indicate that a serialized list of elements of this type is /// separated by commas. @@ -103,7 +229,7 @@ impl OneOrMoreSeparated for UnicodeRange { } impl ToCss for Vec where T: ToCss + OneOrMoreSeparated { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { let mut iter = self.iter(); iter.next().unwrap().to_css(dest)?; for item in iter { @@ -116,14 +242,14 @@ impl ToCss for Vec where T: ToCss + OneOrMoreSeparated { impl ToCss for Box where T: ?Sized + ToCss { fn to_css(&self, dest: &mut W) -> fmt::Result - where W: fmt::Write, + where W: Write, { (**self).to_css(dest) } } impl ToCss for Au { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { write!(dest, "{}px", self.to_f64_px()) } } @@ -131,7 +257,7 @@ impl ToCss for Au { macro_rules! impl_to_css_for_predefined_type { ($name: ty) => { impl<'a> ToCss for $name { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { ::cssparser::ToCss::to_css(self, dest) } }