I also think this is preferable to#60 because it allows us to make this additional change:

diff --git a/lib/syntax_tree/css/selectors.rb b/lib/syntax_tree/css/selectors.rbindex f8dbe6c5..fbae220b 100644--- a/lib/syntax_tree/css/selectors.rb+++ b/lib/syntax_tree/css/selectors.rb@@ -295,7 +295,11 @@ def initialize(tokens)       end       def parse-        selector_list+        selector_list.tap do+          unless EOFToken === tokens.peek+            raise ParseError.new "Unexpected token: #{tokens.peek.value.inspect}"+          end+        end       end

and then a malformed selector likediv::first-line.xl will result in an exception like:

SyntaxTree::CSS::Selectors::ParseError: Unexpected token: "."

The code on the branch in#60 will happily parse this as two compound selectors,[div::first-line, .xl], which is incorrect behavior IMHO.

So I think this is illustrating an issue with the design ofComplexSelector. Previously it was a left, a combinator, and a right. Now it's a flat list. This puts it on the consumer to understand what is implied by the flat list.

Thinking abouthtml > body > main div I think I would prefer that to look like:

(complex-selector  (complex-selector    (complex-selector      (type-selector (wqname (ident-token "html")))      (combinator (delim-token ">"))      (type-selector (wqname (ident-token "body")))    )    (combinator (delim-token ">"))    (type-selector (wqname (ident-token "main")))  )  (combinator (whitespace-token " "))  (type-selector (wqname (ident-token "div"))))

This makes the hierarchy clear, and has the benefit of not requiring any parsing from the consumer. What do you think?

I think I would prefer that to look like ...

You said the opposite in#58 (comment) ! 😆

If you prefer the original way, I'm happy to roll that up into this PR, give me a little bit.

Shit, I totally did. Damnit. Sorry!

The other thing we could do which might make this nicer is we could define types for each combinator instead of having it all be the same thing.

So,DirectDescendentSelector andDescendentSelector andSiblingSelector and so on. That might be a better experience.

define types for each combinator

Yup, I will be sending a PR to do that, but I wanted to sort out this PR first ...

I do slightly prefer the flat list (with proper Combinator types) because there's no operator precedence to worry about that would require proper binary operators.

Do you want me to add a commit introducing Combinator types?

Ahh of course, that was what you were saying earlier. Sorry I must be slow today I'm finally catching up.

Yeah, let's look at what combinator types would look like. At the moment I'm mostly reacting to having to look at "WhitespaceToken" to understand how to handle it, which is making it feel bad.

@kddnewton OK, pushed a commit introducing the subclasses. LMK if you want a different approach (there are obviously alternatives to using constants the way I did)

I'm noticing now that all the combinator classes are dispatched withvisit_combinator, LMK if you think it makes more sense to split it up into multiple methods.