From 0b3a5b42ba209b4527e64d49af6a643cdf8138de Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 4 Mar 2018 23:05:51 +0100 Subject: [PATCH 1/5] Introduce enum CounterBound for the bounds in counter_style::Ranges --- components/style/counter_style/mod.rs | 33 ++++++++++++++++++++------- components/style/gecko/rules.rs | 6 ++--- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/components/style/counter_style/mod.rs b/components/style/counter_style/mod.rs index 0919a9d7619..fa614d476de 100644 --- a/components/style/counter_style/mod.rs +++ b/components/style/counter_style/mod.rs @@ -444,7 +444,18 @@ impl Parse for Negative { /// /// Empty Vec represents 'auto' #[derive(Clone, Debug)] -pub struct Ranges(pub Vec>>); +pub struct Ranges(pub Vec>); + +/// A bound found in `Ranges`. +#[derive(Clone, Copy, Debug)] +pub enum CounterBound { + /// An integer bound. + /// + /// FIXME(https://github.com/servo/servo/issues/20197) + Integer(i32), + /// The infinite bound. + Infinite, +} impl Parse for Ranges { fn parse<'i, 't>(_context: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { @@ -454,7 +465,7 @@ impl Parse for Ranges { input.parse_comma_separated(|input| { let opt_start = parse_bound(input)?; let opt_end = parse_bound(input)?; - if let (Some(start), Some(end)) = (opt_start, opt_end) { + if let (CounterBound::Integer(start), CounterBound::Integer(end)) = (opt_start, opt_end) { if start > end { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -465,11 +476,17 @@ impl Parse for Ranges { } } -fn parse_bound<'i, 't>(input: &mut Parser<'i, 't>) -> Result, ParseError<'i>> { +fn parse_bound<'i, 't>( + input: &mut Parser<'i, 't>, +) -> Result> { let location = input.current_source_location(); match *input.next()? { - Token::Number { int_value: Some(v), .. } => Ok(Some(v)), - Token::Ident(ref ident) if ident.eq_ignore_ascii_case("infinite") => Ok(None), + Token::Number { int_value: Some(v), .. } => { + Ok(CounterBound::Integer(v)) + } + Token::Ident(ref ident) if ident.eq_ignore_ascii_case("infinite") => { + Ok(CounterBound::Infinite) + } ref t => Err(location.new_unexpected_token_error(t.clone())), } } @@ -493,7 +510,7 @@ impl ToCss for Ranges { } } -fn range_to_css(range: &Range>, dest: &mut CssWriter) -> fmt::Result +fn range_to_css(range: &Range, dest: &mut CssWriter) -> fmt::Result where W: Write, { @@ -502,11 +519,11 @@ where bound_to_css(range.end, dest) } -fn bound_to_css(range: Option, dest: &mut CssWriter) -> fmt::Result +fn bound_to_css(range: CounterBound, dest: &mut CssWriter) -> fmt::Result where W: Write, { - if let Some(finite) = range { + if let CounterBound::Integer(finite) = range { finite.to_css(dest) } else { dest.write_str("infinite") diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs index 9460ab2e74b..90d061cd738 100644 --- a/components/style/gecko/rules.rs +++ b/components/style/gecko/rules.rs @@ -6,7 +6,7 @@ use byteorder::{BigEndian, WriteBytesExt}; use computed_values::{font_stretch, font_style, font_weight}; -use counter_style; +use counter_style::{self, CounterBound}; use cssparser::UnicodeRange; use font_face::{FontFaceRuleData, Source, FontDisplay, FontWeight}; use gecko_bindings::bindings; @@ -324,8 +324,8 @@ impl ToNsCssValue for counter_style::Ranges { nscssvalue.set_auto(); } else { nscssvalue.set_pair_list(self.0.into_iter().map(|range| { - fn set_bound(bound: Option, nscssvalue: &mut nsCSSValue) { - if let Some(finite) = bound { + fn set_bound(bound: CounterBound, nscssvalue: &mut nsCSSValue) { + if let CounterBound::Integer(finite) = bound { nscssvalue.set_integer(finite) } else { nscssvalue.set_enum(structs::NS_STYLE_COUNTER_RANGE_INFINITE as i32) From fb8b6fc0a5f4b1ab8e20314a512a20729bc98587 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 4 Mar 2018 23:13:31 +0100 Subject: [PATCH 2/5] Derive ToCss for CounterBound --- components/style/counter_style/mod.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/components/style/counter_style/mod.rs b/components/style/counter_style/mod.rs index fa614d476de..397598d45a7 100644 --- a/components/style/counter_style/mod.rs +++ b/components/style/counter_style/mod.rs @@ -447,7 +447,7 @@ impl Parse for Negative { pub struct Ranges(pub Vec>); /// A bound found in `Ranges`. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, ToCss)] pub enum CounterBound { /// An integer bound. /// @@ -514,20 +514,9 @@ fn range_to_css(range: &Range, dest: &mut CssWriter) -> fmt: where W: Write, { - bound_to_css(range.start, dest)?; + range.start.to_css(dest)?; dest.write_char(' ')?; - bound_to_css(range.end, dest) -} - -fn bound_to_css(range: CounterBound, dest: &mut CssWriter) -> fmt::Result -where - W: Write, -{ - if let CounterBound::Integer(finite) = range { - finite.to_css(dest) - } else { - dest.write_str("infinite") - } + range.end.to_css(dest) } /// From 31036d6510e5d996e730b1db2546a70dab361f4d Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 4 Mar 2018 23:25:40 +0100 Subject: [PATCH 3/5] Derive ToCss for MediaList This uncovered the fancy snowflake `use fmt::Write` in the impl of ToCss for NonTSPseudoClass. --- components/style/gecko/selector_parser.rs | 2 +- components/style/lib.rs | 27 ----------------------- components/style/media_queries.rs | 13 ++--------- 3 files changed, 3 insertions(+), 39 deletions(-) diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index c009dc6b65e..434d2fef414 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -66,7 +66,7 @@ apply_non_ts_list!(pseudo_class_name); impl ToCss for NonTSPseudoClass { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { use cssparser::CssStringWriter; - use fmt::Write; + use std::fmt::Write; macro_rules! pseudo_class_serialize { (bare: [$(($css:expr, $name:ident, $gecko_type:tt, $state:tt, $flags:tt),)*], string: [$(($s_css:expr, $s_name:ident, $s_gecko_type:tt, $s_state:tt, $s_flags:tt),)*]) => { diff --git a/components/style/lib.rs b/components/style/lib.rs index 84ccd32574e..49acbe35f13 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -133,9 +133,6 @@ pub mod traversal_flags; #[allow(non_camel_case_types)] pub mod values; -use std::fmt::{self, Write}; -use style_traits::{CssWriter, ToCss}; - #[cfg(feature = "gecko")] pub use gecko_string_cache as string_cache; #[cfg(feature = "gecko")] pub use gecko_string_cache::Atom; #[cfg(feature = "gecko")] pub use gecko_string_cache::Namespace; @@ -181,30 +178,6 @@ macro_rules! reexport_computed_values { } longhand_properties_idents!(reexport_computed_values); -/// Serializes as CSS a comma-separated list of any `T` that supports being -/// serialized as CSS. -pub fn serialize_comma_separated_list( - dest: &mut CssWriter, - list: &[T], -) -> fmt::Result -where - W: Write, - T: ToCss, -{ - if list.is_empty() { - return Ok(()); - } - - list[0].to_css(dest)?; - - for item in list.iter().skip(1) { - dest.write_str(", ")?; - item.to_css(dest)?; - } - - Ok(()) -} - #[cfg(feature = "gecko")] use gecko_string_cache::WeakAtom; #[cfg(feature = "servo")] use servo_atoms::Atom as WeakAtom; diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs index 8941b21a30a..e6fb4614762 100644 --- a/components/style/media_queries.rs +++ b/components/style/media_queries.rs @@ -13,7 +13,6 @@ use cssparser::{Token, ParserInput}; use error_reporting::{ContextualParseError, ParseErrorReporter}; use parser::{ParserContext, ParserErrorContext}; use selectors::parser::SelectorParseErrorKind; -use serialize_comma_separated_list; use std::fmt::{self, Write}; use str::string_as_ascii_lowercase; use style_traits::{CssWriter, ToCss, ParseError, StyleParseErrorKind}; @@ -25,22 +24,14 @@ pub use servo::media_queries::{Device, Expression}; pub use gecko::media_queries::{Device, Expression}; /// A type that encapsulates a media query list. -#[derive(Clone, Debug)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] +#[css(comma, iterable)] +#[derive(Clone, Debug, ToCss)] pub struct MediaList { /// The list of media queries. pub media_queries: Vec, } -impl ToCss for MediaList { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - serialize_comma_separated_list(dest, &self.media_queries) - } -} - impl MediaList { /// Create an empty MediaList. pub fn empty() -> Self { From 247bcbbbd60dffc120c5ff4b0f4b45f9a8d1259b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 4 Mar 2018 23:35:59 +0100 Subject: [PATCH 4/5] Derive ToCss for all vector longhands that cannot be empty We don't need to check for emptiness and write `none` for them. --- components/style/properties/helpers.mako.rs | 34 +++++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 060d4010e21..6e6c15d981f 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -83,8 +83,10 @@ need_animatable=need_animatable, **kwargs)"> #[allow(unused_imports)] use smallvec::SmallVec; + % if allow_empty: use std::fmt::{self, Write}; use style_traits::{CssWriter, Separator, ToCss}; + % endif pub mod single_value { #[allow(unused_imports)] @@ -122,6 +124,14 @@ % if need_animatable or animation_value_type == "ComputedValue": #[derive(Animate, ComputeSquaredDistance)] % endif + % if not allow_empty: + % if separator == "Comma": + #[css(comma, iterable)] + % else: + #[css(iterable)] + % endif + #[derive(ToCss)] + % endif pub struct T( % if allow_empty and allow_empty != "NotInitial": pub Vec, @@ -154,6 +164,7 @@ } } + % if allow_empty: impl ToCss for computed_value::T { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where @@ -163,11 +174,7 @@ if let Some(val) = iter.next() { val.to_css(dest)?; } else { - % if allow_empty: - dest.write_str("none")?; - % else: - warn!("Found empty value for property ${name}"); - % endif + return dest.write_str("none"); } for i in iter { dest.write_str(::style_traits::${separator}::separator())?; @@ -176,11 +183,21 @@ Ok(()) } } + % endif /// The specified value of ${name}. #[derive(Clone, Debug, MallocSizeOf, PartialEq)] + % if not allow_empty: + % if separator == "Comma": + #[css(comma, iterable)] + % else: + #[css(iterable)] + % endif + #[derive(ToCss)] + % endif pub struct SpecifiedValue(pub Vec); + % if allow_empty: impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where @@ -190,11 +207,7 @@ if let Some(val) = iter.next() { val.to_css(dest)?; } else { - % if allow_empty: - dest.write_str("none")?; - % else: - warn!("Found empty value for property ${name}"); - % endif + return dest.write_str("none"); } for i in iter { dest.write_str(::style_traits::${separator}::separator())?; @@ -203,6 +216,7 @@ Ok(()) } } + % endif pub fn get_initial_value() -> computed_value::T { % if allow_empty and allow_empty != "NotInitial": From 75b05c518d03b1bd8118063e6bca7f02a1e64a06 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 4 Mar 2018 23:48:31 +0100 Subject: [PATCH 5/5] Derive ToCss for DeclaredValueOwned rather than DeclaredValue --- components/style/properties/properties.mako.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 595a9078141..2462d21a715 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1308,7 +1308,7 @@ impl ShorthandId { /// Servo's representation of a declared value for a given `T`, which is the /// declared value for that property. -#[derive(Clone, Debug, Eq, PartialEq, ToCss)] +#[derive(Clone, Debug, Eq, PartialEq)] pub enum DeclaredValue<'a, T: 'a> { /// A known specified value from the stylesheet. Value(&'a T), @@ -1323,7 +1323,7 @@ pub enum DeclaredValue<'a, T: 'a> { /// extra discriminant word) and synthesize dependent DeclaredValues for /// PropertyDeclaration instances as needed. #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, ToCss)] pub enum DeclaredValueOwned { /// A known specified value from the stylesheet. Value(T), @@ -1756,7 +1756,7 @@ impl ToCss for CustomDeclaration { where W: fmt::Write, { - self.value.borrow().to_css(dest) + self.value.to_css(dest) } }