patternjavascriptMinor
PEG.js grammar for parsing CSS selectors
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
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 "
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
Something likes this:
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
Basically it returns a space except for provided empty strings and empty arrays. The code could very well be:
collapse
elsebranches after areturndo not make sense.
- I would propose
map/joininstead ofreduce/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 callBasically 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 callfunction reduceToSingleSpace( x ) {
return ( !x || ( x && x instanceof Array && !x.length ) )?"":" ";
}Context
StackExchange Code Review Q#35487, answer score: 2
Revisions (0)
No revisions yet.