Skip to content

Commit

Permalink
Auto merge of #17414 - jyc:space-separator, r=nox
Browse files Browse the repository at this point in the history
style: Have OneOrMoreSeparated replace OneOrMoreCommaSeparated.

**NOTE** The alternative for me is just to duplicate the ToCss code, which is not bad and is less code changed! Don't know what others would think, though. Looking for feedback!

A future patch series has some values that should be separated by spaces. This
allows us to re-use the code for serialization, but the types do get a little
clunky. The separator is now indicated with an associated type.

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because they shoudl be tested by existing serialization code

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/17414)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Jun 20, 2017
2 parents a7ac921 + 26179b3 commit 40a3826
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 14 deletions.
6 changes: 4 additions & 2 deletions components/style/counter_style/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::ascii::AsciiExt;
use std::borrow::Cow;
use std::fmt;
use std::ops::Range;
use style_traits::{ToCss, OneOrMoreCommaSeparated, ParseError, StyleParseError};
use style_traits::{ToCss, OneOrMoreSeparated, CommaSeparator, ParseError, StyleParseError};
use values::CustomIdent;

/// Parse the prelude of an @counter-style rule
Expand Down Expand Up @@ -552,7 +552,9 @@ pub struct AdditiveTuple {
pub symbol: Symbol,
}

impl OneOrMoreCommaSeparated for AdditiveTuple {}
impl OneOrMoreSeparated for AdditiveTuple {
type S = CommaSeparator;
}

impl Parse for AdditiveTuple {
fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
Expand Down
6 changes: 4 additions & 2 deletions components/style/font_face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use parser::{ParserContext, log_css_error, Parse};
use selectors::parser::SelectorParseError;
use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
use std::fmt;
use style_traits::{ToCss, OneOrMoreCommaSeparated, ParseError, StyleParseError};
use style_traits::{ToCss, OneOrMoreSeparated, CommaSeparator, ParseError, StyleParseError};
use values::specified::url::SpecifiedUrl;

/// A source for a font-face rule.
Expand All @@ -34,7 +34,9 @@ pub enum Source {
Local(FamilyName),
}

impl OneOrMoreCommaSeparated for Source {}
impl OneOrMoreSeparated for Source {
type S = CommaSeparator;
}

/// A `UrlSource` represents a font-face source that has been specified with a
/// `url()` function.
Expand Down
6 changes: 4 additions & 2 deletions components/style/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use context::QuirksMode;
use cssparser::{Parser, SourcePosition, UnicodeRange};
use error_reporting::{ParseErrorReporter, ContextualParseError};
use style_traits::{OneOrMoreCommaSeparated, ParseError, ParsingMode};
use style_traits::{OneOrMoreSeparated, IsCommaSeparator, ParseError, ParsingMode};
#[cfg(feature = "gecko")]
use style_traits::{PARSING_MODE_DEFAULT, PARSING_MODE_ALLOW_UNITLESS_LENGTH, PARSING_MODE_ALLOW_ALL_NUMERIC_VALUES};
use stylesheets::{CssRuleType, Origin, UrlExtraData, Namespaces};
Expand Down Expand Up @@ -161,7 +161,9 @@ pub trait Parse : Sized {
-> Result<Self, ParseError<'i>>;
}

impl<T> Parse for Vec<T> where T: Parse + OneOrMoreCommaSeparated {
impl<T> Parse for Vec<T> where T: Parse + OneOrMoreSeparated,
<T as OneOrMoreSeparated>::S: IsCommaSeparator
{
fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
-> Result<Self, ParseError<'i>> {
input.parse_comma_separated(|input| T::parse(context, input))
Expand Down
6 changes: 4 additions & 2 deletions components/style/values/generics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use counter_style::{Symbols, parse_counter_style_name};
use cssparser::Parser;
use parser::{Parse, ParserContext};
use std::fmt;
use style_traits::{OneOrMoreCommaSeparated, ToCss, ParseError, StyleParseError};
use style_traits::{OneOrMoreSeparated, CommaSeparator, ToCss, ParseError, StyleParseError};
use super::CustomIdent;
use values::specified::url::SpecifiedUrl;

Expand Down Expand Up @@ -123,7 +123,9 @@ pub struct FontSettingTag<T> {
pub value: T,
}

impl<T> OneOrMoreCommaSeparated for FontSettingTag<T> {}
impl<T> OneOrMoreSeparated for FontSettingTag<T> {
type S = CommaSeparator;
}

impl<T: ToCss> ToCss for FontSettingTag<T> {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
Expand Down
2 changes: 1 addition & 1 deletion components/style_traits/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub mod values;
#[macro_use]
pub mod viewport;

pub use values::{ToCss, OneOrMoreCommaSeparated};
pub use values::{ToCss, OneOrMoreSeparated, CommaSeparator, SpaceSeparator, IsCommaSeparator};
pub use viewport::HasViewportPercentage;

/// The error type for all CSS parsing routines.
Expand Down
52 changes: 47 additions & 5 deletions components/style_traits/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,59 @@ impl ToCss for String {
}
}

/// Marker trait to automatically implement ToCss for Vec<T>.
pub trait OneOrMoreCommaSeparated {}
/// 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.
pub struct CommaSeparator;

/// 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 spaces.
pub struct SpaceSeparator;

/// A trait satisfied by the types corresponding to separators.
pub trait Separator {
/// The separator string that the satisfying separator type corresponds to.
fn separator() -> &'static str;
}

impl Separator for CommaSeparator {
fn separator() -> &'static str {
", "
}
}

impl Separator for SpaceSeparator {
fn separator() -> &'static str {
" "
}
}

/// Trait that indicates that satisfying separator types are comma separators.
/// This seems kind of redundant, but we aren't able to express type equality
/// constraints yet.
/// https://github.com/rust-lang/rust/issues/20041
pub trait IsCommaSeparator {}

impl IsCommaSeparator for CommaSeparator {}

impl OneOrMoreCommaSeparated for UnicodeRange {}
/// Marker trait on T to automatically implement ToCss for Vec<T> when T's are
/// separated by some delimiter `delim`.
pub trait OneOrMoreSeparated {
/// Associated type indicating which separator is used.
type S: Separator;
}

impl OneOrMoreSeparated for UnicodeRange {
type S = CommaSeparator;
}

impl<T> ToCss for Vec<T> where T: ToCss + OneOrMoreCommaSeparated {
impl<T> ToCss for Vec<T> where T: ToCss + OneOrMoreSeparated {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
let mut iter = self.iter();
iter.next().unwrap().to_css(dest)?;
for item in iter {
dest.write_str(", ")?;
dest.write_str(<T as OneOrMoreSeparated>::S::separator())?;
item.to_css(dest)?;
}
Ok(())
Expand Down

0 comments on commit 40a3826

Please sign in to comment.