Skip to content

Commit

Permalink
Fix @extend specificity problems
Browse files Browse the repository at this point in the history
Fixes sass#592

Tried to implement specificity as per spec, but this
turned out not to work like ruby sass. Once I tweaked
the specificity constant of all types, it started to
work, but I don't know why ruby sass does it that way!
  • Loading branch information
mgreter committed Apr 1, 2015
1 parent fd7f20f commit d38f278
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 39 deletions.
57 changes: 31 additions & 26 deletions ast.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1742,7 +1742,9 @@ namespace Sass {
{ }
virtual ~Selector() = 0;
// virtual Selector_Placeholder* find_placeholder();
virtual int specificity() { return Constants::SPECIFICITY_BASE; }
virtual unsigned long long specificity() {
return Constants::Specificity_Universal;
};
};
inline Selector::~Selector() { }

Expand Down Expand Up @@ -1787,10 +1789,10 @@ namespace Sass {
Selector_Reference(ParserState pstate, Selector* r = 0)
: Simple_Selector(pstate), selector_(r)
{ has_reference(true); }
virtual int specificity()
virtual unsigned long long specificity()
{
if (selector()) return selector()->specificity();
else return 0;
if (!selector()) return 0;
return selector()->specificity();
}
ATTACH_OPERATIONS();
};
Expand All @@ -1817,10 +1819,11 @@ namespace Sass {
Type_Selector(ParserState pstate, string n)
: Simple_Selector(pstate), name_(n)
{ }
virtual int specificity()
virtual unsigned long long specificity()
{
if (name() == "*") return 0;
else return 1;
// ToDo: What is the specificity of the star selector?
if (name() == "*") return Constants::Specificity_Type;
else return Constants::Specificity_Type;
}
virtual Compound_Selector* unify_with(Compound_Selector*, Context&);
ATTACH_OPERATIONS();
Expand All @@ -1835,10 +1838,11 @@ namespace Sass {
Selector_Qualifier(ParserState pstate, string n)
: Simple_Selector(pstate), name_(n)
{ }
virtual int specificity()
virtual unsigned long long specificity()
{
if (name()[0] == '#') return Constants::SPECIFICITY_BASE * Constants::SPECIFICITY_BASE;
else return Constants::SPECIFICITY_BASE;
if (name()[0] == '#') return Constants::Specificity_ID;
if (name()[0] == '.') return Constants::Specificity_Class;
else return Constants::Specificity_Type;
}
virtual Compound_Selector* unify_with(Compound_Selector*, Context&);
ATTACH_OPERATIONS();
Expand All @@ -1855,6 +1859,10 @@ namespace Sass {
Attribute_Selector(ParserState pstate, string n, string m, String* v)
: Simple_Selector(pstate), name_(n), matcher_(m), value_(v)
{ }
virtual unsigned long long specificity()
{
return Constants::Specificity_Attr;
}
ATTACH_OPERATIONS();
};

Expand All @@ -1868,17 +1876,6 @@ namespace Sass {
Pseudo_Selector(ParserState pstate, string n, String* expr = 0)
: Simple_Selector(pstate), name_(n), expression_(expr)
{ }
virtual int specificity()
{
// TODO: clean up the pseudo-element checking
if (name() == ":before" || name() == "::before" ||
name() == ":after" || name() == "::after" ||
name() == ":first-line" || name() == "::first-line" ||
name() == ":first-letter" || name() == "::first-letter")
return 1;
else
return Constants::SPECIFICITY_BASE;
}
virtual bool is_pseudo_element()
{
if (name() == ":before" || name() == "::before" ||
Expand All @@ -1888,10 +1885,18 @@ namespace Sass {
return true;
}
else {
// If it's not a known pseudo-element, check whether it looks like one. This is similar to the type method on the Pseudo class in ruby sass.
// If it's not a known pseudo-element, check whether it looks like one.
// This is similar to the type method on the Pseudo class in ruby sass.
return name().find("::") == 0;
}
}
virtual unsigned long long specificity()
{
/* needed to match ruby sass */
if (name().find("::") == 0)
return Constants::Specificity_Type;
return Constants::Specificity_Pseudo;
}
virtual Compound_Selector* unify_with(Compound_Selector*, Context&);
ATTACH_OPERATIONS();
};
Expand Down Expand Up @@ -1946,7 +1951,7 @@ namespace Sass {
return 0;
}
bool is_superselector_of(Compound_Selector* rhs);
virtual int specificity()
virtual unsigned long long specificity()
{
int sum = 0;
for (size_t i = 0, L = length(); i < L; ++i)
Expand Down Expand Up @@ -2007,7 +2012,7 @@ namespace Sass {
// virtual Selector_Placeholder* find_placeholder();
Combinator clear_innermost();
void set_innermost(Complex_Selector*, Combinator);
virtual int specificity() const
virtual unsigned long long specificity() const
{
int sum = 0;
if (head()) sum += head()->specificity();
Expand Down Expand Up @@ -2088,9 +2093,9 @@ namespace Sass {
: Selector(pstate), Vectorized<Complex_Selector*>(s), wspace_(0)
{ }
// virtual Selector_Placeholder* find_placeholder();
virtual int specificity()
virtual unsigned long long specificity()
{
int sum = 0;
unsigned long long sum = 0;
for (size_t i = 0, L = length(); i < L; ++i)
{ sum += (*this)[i]->specificity(); }
return sum;
Expand Down
11 changes: 10 additions & 1 deletion constants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,16 @@

namespace Sass {
namespace Constants {
extern const int SPECIFICITY_BASE = 1000;

// https://github.com/sass/libsass/issues/592
// https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity
// https://github.com/sass/sass/issues/1495#issuecomment-61189114
extern const unsigned long long Specificity_Universal = 1 << 0;
extern const unsigned long long Specificity_Type = 1 << 2;
extern const unsigned long long Specificity_Class = 1 << 3;
extern const unsigned long long Specificity_Attr = 1 << 3;
extern const unsigned long long Specificity_Pseudo = 1 << 3;
extern const unsigned long long Specificity_ID = 1 << 10;

// sass keywords
extern const char at_root_kwd[] = "@at-root";
Expand Down
11 changes: 10 additions & 1 deletion constants.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@

namespace Sass {
namespace Constants {
extern const int SPECIFICITY_BASE;

// https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity
// The following list of selectors is by increasing specificity:
extern const unsigned long long Specificity_Universal;
extern const unsigned long long Specificity_Type;
extern const unsigned long long Specificity_Class;
extern const unsigned long long Specificity_Attr;
extern const unsigned long long Specificity_Pseudo;
extern const unsigned long long Specificity_ID;
extern const unsigned long long Specificity_Inline;

// sass keywords
extern const char at_root_kwd[];
Expand Down
27 changes: 18 additions & 9 deletions debugger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define SASS_DEBUGGER_H

#include <string>
#include <sstream>
#include "ast_fwd_decl.hpp"

using namespace std;
Expand All @@ -25,9 +26,15 @@ inline string prettyprint(const string& str) {
return clean;
}

inline string longToHex(long long t) {
std::stringstream is;
is << std::hex << t;
return is.str();
}

inline void debug_ast(AST_Node* node, string ind = "", Env* env = 0)
{

if (node == 0) return;
if (ind == "") cerr << "####################################################################\n";
if (dynamic_cast<Bubble*>(node)) {
Bubble* bubble = dynamic_cast<Bubble*>(node);
Expand Down Expand Up @@ -64,6 +71,7 @@ inline void debug_ast(AST_Node* node, string ind = "", Env* env = 0)
Complex_Selector* selector = dynamic_cast<Complex_Selector*>(node);
cerr << ind << "Complex_Selector " << selector
<< " [block:" << selector->last_block() << "]"
<< " [weight:" << longToHex(selector->specificity()) << "]"
<< (selector->last_block() && selector->last_block()->is_root() ? " [root]" : "")
<< " [@media:" << selector->media_block() << "]"
<< (selector->is_optional() ? " [is_optional]": " -")
Expand All @@ -80,14 +88,15 @@ inline void debug_ast(AST_Node* node, string ind = "", Env* env = 0)
debug_ast(selector->tail(), ind + "-", env);
} else if (dynamic_cast<Compound_Selector*>(node)) {
Compound_Selector* selector = dynamic_cast<Compound_Selector*>(node);
cerr << ind << "Compound_Selector " << selector
<< " [block:" << selector->last_block() << "]"
<< (selector->last_block() && selector->last_block()->is_root() ? " [root]" : "")
<< " [@media:" << selector->media_block() << "]"
<< (selector->is_optional() ? " [is_optional]": " -")
<< (selector->has_line_break() ? " [line-break]": " -")
<< (selector->has_line_feed() ? " [line-feed]": " -") <<
" <" << prettyprint(selector->pstate().token.ws_before()) << ">" << endl;
cerr << ind << "Compound_Selector " << selector;
cerr << " [block:" << selector->last_block() << "]";
cerr << " [weight:" << longToHex(selector->specificity()) << "]";
// cerr << (selector->last_block() && selector->last_block()->is_root() ? " [root]" : "");
cerr << " [@media:" << selector->media_block() << "]";
cerr << (selector->is_optional() ? " [is_optional]": " -");
cerr << (selector->has_line_break() ? " [line-break]": " -");
cerr << (selector->has_line_feed() ? " [line-feed]": " -");
cerr << " <" << prettyprint(selector->pstate().token.ws_before()) << ">" << endl;
for(auto i : selector->elements()) { debug_ast(i, ind + " ", env); }
} else if (dynamic_cast<Propset*>(node)) {
Propset* selector = dynamic_cast<Propset*>(node);
Expand Down
4 changes: 2 additions & 2 deletions extend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -517,14 +517,14 @@ namespace Sass {
// had an extra source that the ruby version did not have. Without a failing test case, this is going to be extra hard to find. My
// best guess at this point is that we're cloning an object somewhere and maintaining the sources when we shouldn't be. This is purely
// a guess though.
int maxSpecificity = 0;
unsigned long long maxSpecificity = 0;
SourcesSet sources = pSeq1->sources();

DEBUG_PRINTLN(TRIM, "TRIMASDF SEQ1: " << seq1)
DEBUG_EXEC(TRIM, printSourcesSet(sources, ctx, "TRIMASDF SOURCES: "))

for (SourcesSet::iterator sourcesSetIterator = sources.begin(), sourcesSetIteratorEnd = sources.end(); sourcesSetIterator != sourcesSetIteratorEnd; ++sourcesSetIterator) {
const Complex_Selector* const pCurrentSelector = *sourcesSetIterator;
const Complex_Selector* const pCurrentSelector = *sourcesSetIterator;
maxSpecificity = max(maxSpecificity, pCurrentSelector->specificity());
}

Expand Down

0 comments on commit d38f278

Please sign in to comment.