Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extract CSS selector cache into CSS::SelectorCache #3226

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

flavorjones
Copy link
Member

What problem is this PR intended to solve?

This is part of a continuing track of work to separate out the concerns of CSS parser, CSS selector cache, and XPath expression translation into distinct components.

In this PR, I've extracted a CSS::SelectorCache class providing functionality that was previously built directly into CSS::Parser (with an awkward API that I'm responsible for writing in 2008).

  • The methods Parser.set_cache, Parser.cache_on?, and Parser.without_cache(&blk) have been removed.
  • The cache is now injected by CSS.xpath_for (optionally, via a cache: keyword argument) instead of being built into the parser.
  • Documentation for CSS.xpath_for has also been updated and improved.

Have you included adequate test coverage?

Mostly an internal refactor, existing test coverage is sufficient except where I removed tests of the removed methods.

Does this change affect the behavior of either the C or the Java implementations?

N/A

The methods `Parser.set_cache`, `Parser.cache_on?`, and
`Parser.without_cache(&blk)` have been removed.

The cache is now injected by `CSS.xpath_for` (optionally, via a
`cache:` keyword argument) instead of being built into the parser.

Documentation for `CSS.xpath_for` has also been updated and improved.
@flavorjones flavorjones merged commit 409e041 into main Jun 11, 2024
131 of 132 checks passed
@flavorjones flavorjones deleted the flavorjones-extract-css-selector-cache branch June 11, 2024 18:26
@flavorjones flavorjones added this to the v1.17.0 milestone Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant