Tidy the CSS parser API #3218
Closed
Tidy the CSS parser API #3218
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flavorjones
force-pushed
the
flavorjones-tidy-the-css-parser-api
branch
from
e4a6a24
to
c6cb68d
Compare
so that I can run them all with `-n/Nokogiri::CSS/` while I'm developing the SyntaxTree integration.
Previously we've been handling it as a separate input to the CSS.xpath_for method. This simplifies the code and the parser cache key.
flavorjones
force-pushed
the
flavorjones-tidy-the-css-parser-api
branch
from
c6cb68d
to
e208053
Compare
The global `Parser.set_cache` and `Parser.cache_on?` methods have been removed, as has the `Parser.without_cache(&blk)` method. The cache is now used by `CSS.xpath_for` (optionally, via a `cache:` kwarg) instead of being part of the parser. CSS::Parser has a different API which accepts the selector as a parameter to the constructor (and `#parse` no longer takes an argument). This is a prefactor for work to come.
flavorjones
force-pushed
the
flavorjones-tidy-the-css-parser-api
branch
from
e208053
to
35104b5
Compare
|
This PR is too big, and some of these changes I want to land in v1.17 and some may land in a later release. I'm going to break it up into smaller PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem is this PR intended to solve?
In preparation for #2560's work to introduce a new CSS selector parser, I want to take the opportunity to clean some things up, including the awkward cache API that I wrote in (gulp) 2008.
None of this should be considered a breaking change, since the
CSS::Parserand related classes are undocumented and considered internal-only (with the notable exception ofCSS::XPathVisitor).Big changes in this PR:
CSS::SelectorCacheCSS.xpath_fortakes acache:kwarg and this is where the caching is done (not in the Parser)CSS.set_cache,CSS.cache_on?andCSS.without_cache(&blk). Using thecache:argument toCSS.xpath_forshould be sufficient and is in fact clearer code to write.prefixparameter for CSS-to-XPath transpilation is now pushed into theXPathVisitorclass rather than being handled as an additional argument.CSS::ParserAPI has changed: the selector is now passed to.newand not#parse.Smaller changes:
CSS.xpath_fordeprecates the options hash and emits a warning. Users should prefer keyword arguments.CSS.parseis deprecated and emits a warnings. (I will probably remove this in the parser rewrite since it returns an AST that is not supported.)XPathVisitorexposes#prefix,#builtins, and#doctypeattributes.CSS.xpath_for.@tenderlove I would love your eyeballs on this if you get a few minutes.
Have you included adequate test coverage?
Yes.
Does this change affect the behavior of either the C or the Java implementations?
No native behavior changes.