HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

PEG.js grammar for parsing CSS selectors

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
cssselectorsparsinggrammarforpeg

Problem

I have a library that's parsing some expressions, part of which is a CSS selector. More accurately, it's a jQuery-compatible selector. The selector itself is opaque to my library - I don't need to pick it apart, and I don't need to verify that tag and attribute names are valid HTML, or that its pseudo-selectors exist in the current spec. For now, I just need it to match all syntactically valid CSS selectors and not match the rest of the expression, which is delimited by characters that would not be valid in a selector unless escaped or put in a string in an attribute selector. I would use a RegExp and split the string if CSS had no escapes or quoted values, but it started looking write-only really fast.

Does this PEG.js grammar cover all the other edge cases as defined by the CSS standard and jQuery documentation? Could it be more readable? Better in some other way?

```
{
/*
* glom the array contents returned by the parser
* into a string recursively.
*/
function collapse(stuff) {
if (typeof stuff === "string") {
return stuff;
} else if (stuff instanceof Array) {
return stuff.reduce(function (a, item) {
return a.concat(collapse(item));
}, "")
} else {
return "";
}
}

/*
* Replace superfluous white space with a single space.
*/
function trimWs(present) {
if (present) {
if (present instanceof Array) {
return !!present.length ? " " : "";
} else {
return " ";
}
} else {
return "";
}
}

}

start = jqSelector

ws "white space" =
whites:(" " / "\r" / "\n" / "\t" / "\f")+
{ return collapse(whites); }

iws "ignored white space" = ws? { return ""; }
cws "collapsed white space" = ws:ws { return trimWs(ws); }

jqSelector "jquery-compatible selector" =
element:(tagIdClassSelector / cssFunctional / cssAttrExpr)+
more:(iws [\+>~,] iws jqSelector / cws jqSelector)?
{ return collapse([element, more]); }

tagIdClassSelector "

Solution

I can only review the JS part, I am not sure that grammar reviews are part of CR.

collapse

  • else branches after a return do not make sense.



  • I would propose map/join instead of reduce/concat



Something likes this:

function collapse(stuff) {
    if (typeof stuff === "string")
        return stuff;
    if (stuff instanceof Array) 
        return stuff.map(function ( value ) {
          return collapse( value );
        }).join("");
    return "";
}


trimWs

The function does not match the comment, and I cannot see what it is supposed to do. You need a better function name, a better parameter name and a proper comment explaining what it does and how it is used.

Update

trimWs( "abc" ) -> Returns " "
trimWs( "  " ) -> Returns " "
trimWs( " " ) -> Returns " "
trimWs( "\t" ) -> Returns " "
trimWs( "" ) -> Returns ""
trimWs( "   abc   " ) -> Returns " "
If this function were trimming, it would return "abc" for the last call


Basically it returns a space except for provided empty strings and empty arrays. The code could very well be:

function reduceToSingleSpace( x ) {
  return ( !x || ( x && x instanceof Array && !x.length ) )?"":" ";
}

Code Snippets

function collapse(stuff) {
    if (typeof stuff === "string")
        return stuff;
    if (stuff instanceof Array) 
        return stuff.map(function ( value ) {
          return collapse( value );
        }).join("");
    return "";
}
trimWs( "abc" ) -> Returns " "
trimWs( "  " ) -> Returns " "
trimWs( " " ) -> Returns " "
trimWs( "\t" ) -> Returns " "
trimWs( "" ) -> Returns ""
trimWs( "   abc   " ) -> Returns " "
If this function were trimming, it would return "abc" for the last call
function reduceToSingleSpace( x ) {
  return ( !x || ( x && x instanceof Array && !x.length ) )?"":" ";
}

Context

StackExchange Code Review Q#35487, answer score: 2

Revisions (0)

No revisions yet.