Skip to content

Commit

Permalink
Improve code after PR reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
@mgreter
mgreter committed Jun 15, 2019
1 parent af46297 commit 0de2276
Show file tree
Hide file tree
Showing 15 changed files with 289 additions and 241 deletions.
16 changes: 8 additions & 8 deletions src/ast.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@ namespace Sass {

Block::Block(ParserState pstate, size_t s, bool r)
: Statement(pstate),
Vectorized<Statement_Obj, Block>(s),
Vectorized<Statement_Obj>(s),
is_root_(r)
{ }
Block::Block(const Block* ptr)
: Statement(ptr),
Vectorized<Statement_Obj, Block>(*ptr),
Vectorized<Statement_Obj>(*ptr),
is_root_(ptr->is_root_)
{ }

Expand Down Expand Up @@ -659,14 +659,14 @@ namespace Sass {

Arguments::Arguments(ParserState pstate)
: Expression(pstate),
Vectorized<Argument_Obj, Arguments>(),
Vectorized<Argument_Obj>(),
has_named_arguments_(false),
has_rest_argument_(false),
has_keyword_argument_(false)
{ }
Arguments::Arguments(const Arguments* ptr)
: Expression(ptr),
Vectorized<Argument_Obj, Arguments>(*ptr),
Vectorized<Argument_Obj>(*ptr),
has_named_arguments_(ptr->has_named_arguments_),
has_rest_argument_(ptr->has_rest_argument_),
has_keyword_argument_(ptr->has_keyword_argument_)
Expand Down Expand Up @@ -741,12 +741,12 @@ namespace Sass {
/////////////////////////////////////////////////////////////////////////

Media_Query::Media_Query(ParserState pstate, String_Obj t, size_t s, bool n, bool r)
: Expression(pstate), Vectorized<Media_Query_Expression_Obj, Media_Query>(s),
: Expression(pstate), Vectorized<Media_Query_Expression_Obj>(s),
media_type_(t), is_negated_(n), is_restricted_(r)
{ }
Media_Query::Media_Query(const Media_Query* ptr)
: Expression(ptr),
Vectorized<Media_Query_Expression_Obj, Media_Query>(*ptr),
Vectorized<Media_Query_Expression_Obj>(*ptr),
media_type_(ptr->media_type_),
is_negated_(ptr->is_negated_),
is_restricted_(ptr->is_restricted_)
Expand Down Expand Up @@ -872,13 +872,13 @@ namespace Sass {

Parameters::Parameters(ParserState pstate)
: AST_Node(pstate),
Vectorized<Parameter_Obj, Parameters>(),
Vectorized<Parameter_Obj>(),
has_optional_parameters_(false),
has_rest_parameter_(false)
{ }
Parameters::Parameters(const Parameters* ptr)
: AST_Node(ptr),
Vectorized<Parameter_Obj, Parameters>(*ptr),
Vectorized<Parameter_Obj>(*ptr),
has_optional_parameters_(ptr->has_optional_parameters_),
has_rest_parameter_(ptr->has_rest_parameter_)
{ }
Expand Down
45 changes: 21 additions & 24 deletions src/ast.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ namespace Sass {
// "Template Method" design pattern to allow subclasses to adjust their flags
// when certain objects are pushed.
/////////////////////////////////////////////////////////////////////////////
template <typename T, typename U>
template <typename T>
class Vectorized {
std::vector<T> elements_;
protected:
Expand All @@ -218,24 +218,25 @@ namespace Sass {
public:
Vectorized(size_t s = 0) : hash_(0)
{ elements_.reserve(s); }
Vectorized(const std::vector<T> vec) : hash_(0)
{ elements_ = vec; }
Vectorized(std::vector<T> vec) :
elements_(std::move(vec)),
hash_(0)
{}
virtual ~Vectorized() = 0;
size_t length() const { return elements_.size(); }
bool empty() const { return elements_.empty(); }
void clear() { return elements_.clear(); }
T last() const { return elements_.back(); }
T first() const { return elements_.front(); }

bool operator== (const Vectorized<T,U>& rhs) const {
bool operator== (const Vectorized<T>& rhs) const {
// Abort early if sizes do not match
if (length() != rhs.length()) return false;
// Otherwise test each ast node in order
return std::equal(begin(), end(), rhs.begin(),
ObjEqualityFn<CssMediaQuery_Obj>);
// Otherwise test each node for object equalicy in order
return std::equal(begin(), end(), rhs.begin(), ObjEqualityFn<T>);
}

bool operator!= (const Vectorized<T,U>& rhs) const {
bool operator!= (const Vectorized<T>& rhs) const {
return !(*this == rhs);
}

Expand All @@ -261,28 +262,25 @@ namespace Sass {
const std::vector<T>& elements() const { return elements_; }

// Insert all items from compatible vector
virtual U* concat(const std::vector<T>& v)
void concat(const std::vector<T>& v)
{
if (!v.empty()) reset_hash();
elements().insert(end(), v.begin(), v.end());
return static_cast<U*>(this);
}

// Syntatic sugar for pointers
virtual U* concat(Vectorized<T, U>* v)
void concat(Vectorized<T>* v)
{
if (v != nullptr) return concat(*v);
else return static_cast<U*>(this);
}

// Insert one item on the front
virtual U* unshift(T element)
void unshift(T element)
{
if (element) {
reset_hash();
elements_.insert(begin(), element);
}
return static_cast<U*>(this);
}

// Remove and return item on the front
Expand All @@ -296,7 +294,7 @@ namespace Sass {

// Insert one item on the back
// ToDo: rename this to push
virtual U* append(T element)
void append(T element)
{
if (element) {
reset_hash();
Expand All @@ -305,7 +303,6 @@ namespace Sass {
// ToDo: Find a more elegant way to support this
adjust_after_pushing(element);
}
return static_cast<U*>(this);
}

// Check if an item already exists
Expand Down Expand Up @@ -353,8 +350,8 @@ namespace Sass {
typename std::vector<T>::const_iterator erase(typename std::vector<T>::const_iterator el) { reset_hash(); return elements_.erase(el); }

};
template <typename T, typename U>
inline Vectorized<T, U>::~Vectorized() { }
template <typename T>
inline Vectorized<T>::~Vectorized() { }

/////////////////////////////////////////////////////////////////////////////
// Mixin class for AST nodes that should behave like a hash table. Uses an
Expand Down Expand Up @@ -503,7 +500,7 @@ namespace Sass {
////////////////////////
// Blocks of statements.
////////////////////////
class Block final : public Statement, public Vectorized<Statement_Obj, Block> {
class Block final : public Statement, public Vectorized<Statement_Obj> {
ADD_PROPERTY(bool, is_root)
// needed for properly formatted CSS emission
protected:
Expand Down Expand Up @@ -868,7 +865,7 @@ namespace Sass {
// error checking (e.g., ensuring that all ordinal arguments precede all
// named arguments).
////////////////////////////////////////////////////////////////////////
class Arguments final : public Expression, public Vectorized<Argument_Obj, Arguments> {
class Arguments final : public Expression, public Vectorized<Argument_Obj> {
ADD_PROPERTY(bool, has_named_arguments)
ADD_PROPERTY(bool, has_rest_argument)
ADD_PROPERTY(bool, has_keyword_argument)
Expand Down Expand Up @@ -900,7 +897,7 @@ namespace Sass {
// A Media Ruleset after it has been evaluated
// Representing the static or resulting css
class CssMediaRule final : public Has_Block,
public Vectorized<CssMediaQuery_Obj, CssMediaRule> {
public Vectorized<CssMediaQuery_Obj> {
public:
CssMediaRule(ParserState pstate, Block_Obj b);
bool bubbles() override { return true; };
Expand Down Expand Up @@ -962,7 +959,7 @@ namespace Sass {

// Whether this media query matches all media types.
bool matchesAllTypes() const {
return type_.empty() || equalsLiteral("all", type_);
return type_.empty() || Util::equalsLiteral("all", type_);
}

// Merges this with [other] and adds a query that matches the intersection
Expand All @@ -979,7 +976,7 @@ namespace Sass {
// ToDo: only used for interpolation case
////////////////////////////////////////////////////
class Media_Query final : public Expression,
public Vectorized<Media_Query_Expression_Obj, Media_Query> {
public Vectorized<Media_Query_Expression_Obj> {
ADD_PROPERTY(String_Obj, media_type)
ADD_PROPERTY(bool, is_negated)
ADD_PROPERTY(bool, is_restricted)
Expand Down Expand Up @@ -1048,7 +1045,7 @@ namespace Sass {
// error checking (e.g., ensuring that all optional parameters follow all
// required parameters).
/////////////////////////////////////////////////////////////////////////
class Parameters final : public AST_Node, public Vectorized<Parameter_Obj, Parameters> {
class Parameters final : public AST_Node, public Vectorized<Parameter_Obj> {
ADD_PROPERTY(bool, has_optional_parameters)
ADD_PROPERTY(bool, has_rest_parameter)
protected:
Expand Down
97 changes: 20 additions & 77 deletions src/ast_helpers.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// __EXTENSIONS__ fix on Solaris.
#include "sass.hpp"
#include <algorithm>
#include "util_string.hpp"

namespace Sass {

Expand Down Expand Up @@ -45,8 +46,6 @@ namespace Sass {

template <class T>
bool PtrEqualityFn(const T* lhs, const T* rhs) {
if (lhs == nullptr) return rhs == nullptr;
if (rhs == nullptr) return lhs == nullptr;
return lhs == rhs; // compare raw pointers
}

Expand Down Expand Up @@ -216,47 +215,24 @@ namespace Sass {
// Note: this works by comparing the raw pointers
template <typename T>
bool listIsSubsetOrEqual(const T& lhs, const T& rhs) {
for (auto const& item : lhs) {
for (const auto& item : lhs) {
if (std::find(rhs.begin(), rhs.end(), item) == rhs.end())
return false;
}
return true;
}

// ##########################################################################
// Special case insensitive string matcher. We can optimize
// the more general compare case quite a bit by requiring
// consumers to obey some rules (lowercase and no space).
// - `literal` must only contain lower case ascii characters
// there is one edge case where this could give false positives
// test could contain a (non-ascii) char exactly 32 below literal
// ##########################################################################
inline bool equalsLiteral(const std::string& literal, const std::string test) {
// Work directly on characters
const char* src = test.c_str();
const char* lit = literal.c_str();
// There is a small chance that the search string
// Is longer than the rest of the string to look at
while (*lit && (*src == *lit || *src + 32 == *lit)) {
++src, ++lit;
}
// True if literal is at end
// If not test was too long
return *lit == 0;
}
// EO equalsLiteral

// ##########################################################################
// Returns whether [name] is the name of a pseudo-element
// that can be written with pseudo-class syntax (CSS2 vs CSS3):
// `:before`, `:after`, `:first-line`, or `:first-letter`
// ##########################################################################
inline bool isFakePseudoElement(const std::string& name)
{
return equalsLiteral("after", name)
|| equalsLiteral("before", name)
|| equalsLiteral("first-line", name)
|| equalsLiteral("first-letter", name);
return Util::equalsLiteral("after", name)
|| Util::equalsLiteral("before", name)
|| Util::equalsLiteral("first-line", name)
|| Util::equalsLiteral("first-letter", name);
}

// ##########################################################################
Expand All @@ -266,10 +242,10 @@ namespace Sass {
// ##########################################################################
inline bool isSubselectorPseudo(const std::string& norm)
{
return equalsLiteral("any", norm)
|| equalsLiteral("matches", norm)
|| equalsLiteral("nth-child", norm)
|| equalsLiteral("nth-last-child", norm);
return Util::equalsLiteral("any", norm)
|| Util::equalsLiteral("matches", norm)
|| Util::equalsLiteral("nth-child", norm)
|| Util::equalsLiteral("nth-last-child", norm);
}
// EO isSubselectorPseudo

Expand All @@ -278,13 +254,13 @@ namespace Sass {
// ###########################################################################
inline bool isSelectorPseudoClass(const std::string& test)
{
return equalsLiteral("not", test)
|| equalsLiteral("matches", test)
|| equalsLiteral("current", test)
|| equalsLiteral("any", test)
|| equalsLiteral("has", test)
|| equalsLiteral("host", test)
|| equalsLiteral("host-context", test);
return Util::equalsLiteral("not", test)
|| Util::equalsLiteral("matches", test)
|| Util::equalsLiteral("current", test)
|| Util::equalsLiteral("any", test)
|| Util::equalsLiteral("has", test)
|| Util::equalsLiteral("host", test)
|| Util::equalsLiteral("host-context", test);
}
// EO isSelectorPseudoClass

Expand All @@ -293,7 +269,7 @@ namespace Sass {
// ###########################################################################
inline bool isSelectorPseudoElement(const std::string& test)
{
return equalsLiteral("slotted", test);
return Util::equalsLiteral("slotted", test);
}
// EO isSelectorPseudoElement

Expand All @@ -302,44 +278,11 @@ namespace Sass {
// ###########################################################################
inline bool isSelectorPseudoBinominal(const std::string& test)
{
return equalsLiteral("nth-child", test)
|| equalsLiteral("nth-last-child", test);
return Util::equalsLiteral("nth-child", test)
|| Util::equalsLiteral("nth-last-child", test);
}
// isSelectorPseudoBinominal

// ###########################################################################
// Returns [name] without a vendor prefix.
// If [name] has no vendor prefix, it's returned as-is.
// ###########################################################################
inline std::string unvendor(const std::string& name)
{
if (name.size() < 2) return name;
if (name[0] != '-') return name;
if (name[1] == '-') return name;
for (size_t i = 2; i < name.size(); i++) {
if (name[i] == '-') return name.substr(i + 1);
}
return name;
}
// EO unvendor

// ###########################################################################
// Return [name] without pseudo and vendor prefix
// ###########################################################################
inline std::string pseudoName(std::string name)
{
size_t n = name[0] == ':' ? 1 : 0;
std::replace(name.begin(), name.end(), '_', '-');
if (name.size() < 2 + n) return name.substr(n);
if (name[n] != '-') return name.substr(n);
if (name[n + 1] == '-') return name.substr(n);
for (size_t i = 2 + n; i < name.size(); i++) {
if (name[i] == '-') return name.substr(i + 1);
}
return name.substr(n);
}
// EO pseudoName

// ###########################################################################
// ###########################################################################

Expand Down
Loading

0 comments on commit 0de2276

Please sign in to comment.