From 705ecb4557e1dc25c0753f080b76b227a7f0169f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 6 Apr 2017 13:24:38 +0200 Subject: [PATCH] style: Multiple style nits across parsing code. --- .../style/properties/longhand/font.mako.rs | 20 ++++--- .../longhand/inherited_text.mako.rs | 48 +++++++-------- .../properties/shorthand/position.mako.rs | 4 +- components/style/values/specified/grid.rs | 53 +++++++++-------- components/style/values/specified/length.rs | 58 ++++++++++--------- 5 files changed, 98 insertions(+), 85 deletions(-) diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index abceff2dcd5..a300fae7900 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -659,15 +659,17 @@ ${helpers.single_keyword("font-variant-caps", /// | | | pub fn parse(_: &ParserContext, input: &mut Parser) -> Result { if let Ok(lop) = input.try(specified::LengthOrPercentage::parse_non_negative) { - Ok(SpecifiedValue::Length(lop)) - } else if let Ok(kw) = input.try(KeywordSize::parse) { - Ok(SpecifiedValue::Keyword(kw)) - } else { - match_ignore_ascii_case! {&*input.expect_ident()?, - "smaller" => Ok(SpecifiedValue::Smaller), - "larger" => Ok(SpecifiedValue::Larger), - _ => Err(()) - } + return Ok(SpecifiedValue::Length(lop)) + } + + if let Ok(kw) = input.try(KeywordSize::parse) { + return Ok(SpecifiedValue::Keyword(kw)) + } + + match_ignore_ascii_case! {&*input.expect_ident()?, + "smaller" => Ok(SpecifiedValue::Smaller), + "larger" => Ok(SpecifiedValue::Larger), + _ => Err(()) } } diff --git a/components/style/properties/longhand/inherited_text.mako.rs b/components/style/properties/longhand/inherited_text.mako.rs index baf8e0ecb8d..47075cbc8cd 100644 --- a/components/style/properties/longhand/inherited_text.mako.rs +++ b/components/style/properties/longhand/inherited_text.mako.rs @@ -45,31 +45,33 @@ } } /// normal | | | - pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + pub fn parse(_: &ParserContext, input: &mut Parser) -> Result { use cssparser::Token; use std::ascii::AsciiExt; - // We try to parse as a Number first because, for 'line-height', we want "0" to be - // parsed as a plain Number rather than a Length (0px); this matches the behaviour - // of all major browsers - input.try(specified::Number::parse_non_negative) - .map(SpecifiedValue::Number) - .or_else(|()| { - input.try(specified::LengthOrPercentage::parse_non_negative) - .map(SpecifiedValue::LengthOrPercentage) - .or_else(|()| { - match try!(input.next()) { - Token::Ident(ref value) if value.eq_ignore_ascii_case("normal") => { - Ok(SpecifiedValue::Normal) - } - % if product == "gecko": - Token::Ident(ref value) if value.eq_ignore_ascii_case("-moz-block-height") => { - Ok(SpecifiedValue::MozBlockHeight) - } - % endif - _ => Err(()), - } - }) - }) + + // We try to parse as a Number first because, for 'line-height', we want + // "0" to be parsed as a plain Number rather than a Length (0px); this + // matches the behaviour of all major browsers + if let Ok(number) = input.try(specified::Number::parse_non_negative) { + return Ok(SpecifiedValue::Number(number)) + } + + if let Ok(lop) = input.try(specified::LengthOrPercentage::parse_non_negative) { + return Ok(SpecifiedValue::LengthOrPercentage(lop)) + } + + + match try!(input.next()) { + Token::Ident(ref value) if value.eq_ignore_ascii_case("normal") => { + Ok(SpecifiedValue::Normal) + } + % if product == "gecko": + Token::Ident(ref value) if value.eq_ignore_ascii_case("-moz-block-height") => { + Ok(SpecifiedValue::MozBlockHeight) + } + % endif + _ => Err(()), + } } pub mod computed_value { use app_units::Au; diff --git a/components/style/properties/shorthand/position.mako.rs b/components/style/properties/shorthand/position.mako.rs index 825a63d3478..3a777365687 100644 --- a/components/style/properties/shorthand/position.mako.rs +++ b/components/style/properties/shorthand/position.mako.rs @@ -56,8 +56,8 @@ use values::specified::LengthOrPercentageOrAutoOrContent; % endif - pub fn parse_flexibility(input: &mut Parser) - -> Result<(Number, Option),()> { + fn parse_flexibility(input: &mut Parser) + -> Result<(Number, Option),()> { let grow = try!(Number::parse_non_negative(input)); let shrink = input.try(Number::parse_non_negative).ok(); Ok((grow, shrink)) diff --git a/components/style/values/specified/grid.rs b/components/style/values/specified/grid.rs index 5ceb16dd889..e04e85b9394 100644 --- a/components/style/values/specified/grid.rs +++ b/components/style/values/specified/grid.rs @@ -6,6 +6,7 @@ use cssparser::{Parser, Token}; use parser::{Parse, ParserContext}; +use std::ascii::AsciiExt; use std::fmt; use style_traits::ToCss; use values::{CSSFloat, HasViewportPercentage}; @@ -130,23 +131,23 @@ pub enum TrackBreadth { /// Parse a single flexible length. pub fn parse_flex(input: &mut Parser) -> Result { match try!(input.next()) { - Token::Dimension(ref value, ref unit) if unit.to_lowercase() == "fr" && value.value.is_sign_positive() + Token::Dimension(ref value, ref unit) if unit.eq_ignore_ascii_case("fr") && value.value.is_sign_positive() => Ok(value.value), _ => Err(()), } } impl Parse for TrackBreadth { - fn parse(_context: &ParserContext, input: &mut Parser) -> Result { + fn parse(_: &ParserContext, input: &mut Parser) -> Result { if let Ok(lop) = input.try(LengthOrPercentage::parse_non_negative) { - Ok(TrackBreadth::Breadth(lop)) - } else { - if let Ok(f) = input.try(parse_flex) { - Ok(TrackBreadth::Flex(f)) - } else { - TrackKeyword::parse(input).map(TrackBreadth::Keyword) - } + return Ok(TrackBreadth::Breadth(lop)) } + + if let Ok(f) = input.try(parse_flex) { + return Ok(TrackBreadth::Flex(f)) + } + + TrackKeyword::parse(input).map(TrackBreadth::Keyword) } } @@ -223,24 +224,28 @@ impl Default for TrackSize { impl Parse for TrackSize { fn parse(context: &ParserContext, input: &mut Parser) -> Result { if let Ok(b) = input.try(|i| TrackBreadth::parse(context, i)) { - Ok(TrackSize::Breadth(b)) - } else { - if input.try(|i| i.expect_function_matching("minmax")).is_ok() { - Ok(try!(input.parse_nested_block(|input| { - let inflexible_breadth = if let Ok(lop) = input.try(LengthOrPercentage::parse_non_negative) { - Ok(TrackBreadth::Breadth(lop)) - } else { - TrackKeyword::parse(input).map(TrackBreadth::Keyword) + return Ok(TrackSize::Breadth(b)) + } + + if input.try(|i| i.expect_function_matching("minmax")).is_ok() { + return input.parse_nested_block(|input| { + let inflexible_breadth = + match input.try(LengthOrPercentage::parse_non_negative) { + Ok(lop) => TrackBreadth::Breadth(lop), + Err(..) => { + let keyword = try!(TrackKeyword::parse(input)); + TrackBreadth::Keyword(keyword) + } }; - try!(input.expect_comma()); - Ok(TrackSize::MinMax(try!(inflexible_breadth), try!(TrackBreadth::parse(context, input)))) - }))) - } else { - try!(input.expect_function_matching("fit-content")); - Ok(try!(LengthOrPercentage::parse(context, input).map(TrackSize::FitContent))) - } + try!(input.expect_comma()); + Ok(TrackSize::MinMax(inflexible_breadth, try!(TrackBreadth::parse(context, input)))) + }); } + + try!(input.expect_function_matching("fit-content")); + // FIXME(emilio): This needs a parse_nested_block, doesn't it? + Ok(try!(LengthOrPercentage::parse(context, input).map(TrackSize::FitContent))) } } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index ed31cc572e8..f647eafef9c 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -442,7 +442,7 @@ impl Length { /// Parse a non-negative length #[inline] pub fn parse_non_negative(input: &mut Parser) -> Result { - Length::parse_internal(input, AllowedNumericType::NonNegative) + Self::parse_internal(input, AllowedNumericType::NonNegative) } /// Get an absolute length from a px value. @@ -462,7 +462,7 @@ impl Length { impl Parse for Length { fn parse(_context: &ParserContext, input: &mut Parser) -> Result { - Length::parse_internal(input, AllowedNumericType::All) + Self::parse_internal(input, AllowedNumericType::All) } } @@ -1049,7 +1049,7 @@ impl LengthOrPercentage { /// Parse a non-negative length. #[inline] pub fn parse_non_negative(input: &mut Parser) -> Result { - LengthOrPercentage::parse_internal(input, AllowedNumericType::NonNegative) + Self::parse_internal(input, AllowedNumericType::NonNegative) } /// Parse a length, treating dimensionless numbers as pixels @@ -1057,11 +1057,13 @@ impl LengthOrPercentage { /// https://www.w3.org/TR/SVG2/types.html#presentation-attribute-css-value pub fn parse_numbers_are_pixels(input: &mut Parser) -> Result { if let Ok(lop) = input.try(|i| Self::parse_internal(i, AllowedNumericType::All)) { - Ok(lop) - } else { - let num = input.expect_number()?; - Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32)))) + return Ok(lop) } + + // TODO(emilio): Probably should use Number::parse_non_negative to + // handle calc()? + let num = input.expect_number()?; + Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32)))) } /// Parse a non-negative length, treating dimensionless numbers as pixels @@ -1069,14 +1071,16 @@ impl LengthOrPercentage { /// This is nonstandard behavior used by Firefox for SVG pub fn parse_numbers_are_pixels_non_negative(input: &mut Parser) -> Result { if let Ok(lop) = input.try(|i| Self::parse_internal(i, AllowedNumericType::NonNegative)) { - Ok(lop) + return Ok(lop) + } + + // TODO(emilio): Probably should use Number::parse_non_negative to + // handle calc()? + let num = input.expect_number()?; + if num >= 0. { + Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32)))) } else { - let num = input.expect_number()?; - if num >= 0. { - Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32)))) - } else { - Err(()) - } + Err(()) } } @@ -1092,7 +1096,7 @@ impl LengthOrPercentage { impl Parse for LengthOrPercentage { #[inline] fn parse(_context: &ParserContext, input: &mut Parser) -> Result { - LengthOrPercentage::parse_internal(input, AllowedNumericType::All) + Self::parse_internal(input, AllowedNumericType::All) } } @@ -1168,7 +1172,7 @@ impl LengthOrPercentageOrAuto { /// Parse a non-negative length, percentage, or auto. #[inline] pub fn parse_non_negative(input: &mut Parser) -> Result { - LengthOrPercentageOrAuto::parse_internal(input, AllowedNumericType::NonNegative) + Self::parse_internal(input, AllowedNumericType::NonNegative) } /// Parse a non-negative length, percentage, or auto. @@ -1183,7 +1187,7 @@ impl LengthOrPercentageOrAuto { impl Parse for LengthOrPercentageOrAuto { #[inline] fn parse(_context: &ParserContext, input: &mut Parser) -> Result { - LengthOrPercentageOrAuto::parse_internal(input, AllowedNumericType::All) + Self::parse_internal(input, AllowedNumericType::All) } } @@ -1241,15 +1245,15 @@ impl LengthOrPercentageOrNone { } /// Parse a non-negative LengthOrPercentageOrNone. #[inline] - pub fn parse_non_negative(input: &mut Parser) -> Result { - LengthOrPercentageOrNone::parse_internal(input, AllowedNumericType::NonNegative) + pub fn parse_non_negative(input: &mut Parser) -> Result { + Self::parse_internal(input, AllowedNumericType::NonNegative) } } impl Parse for LengthOrPercentageOrNone { #[inline] fn parse(_context: &ParserContext, input: &mut Parser) -> Result { - LengthOrPercentageOrNone::parse_internal(input, AllowedNumericType::All) + Self::parse_internal(input, AllowedNumericType::All) } } @@ -1341,15 +1345,15 @@ pub type LengthOrNumber = Either; impl LengthOrNumber { /// Parse a non-negative LengthOrNumber. - pub fn parse_non_negative(_context: &ParserContext, input: &mut Parser) -> Result { - // We try to parse as a Number first because, for cases like LengthOrNumber, - // we want "0" to be parsed as a plain Number rather than a Length (0px); this - // matches the behaviour of all major browsers + pub fn parse_non_negative(_: &ParserContext, input: &mut Parser) -> Result { + // We try to parse as a Number first because, for cases like + // LengthOrNumber, we want "0" to be parsed as a plain Number rather + // than a Length (0px); this matches the behaviour of all major browsers if let Ok(v) = input.try(Number::parse_non_negative) { - Ok(Either::Second(v)) - } else { - Length::parse_non_negative(input).map(Either::First) + return Ok(Either::Second(v)) } + + Length::parse_non_negative(input).map(Either::First) } }