Skip to content

Conversation

@dlh01
Copy link
Member

@dlh01 dlh01 commented Mar 1, 2025

Summary

As titled.

Notes for reviewers

Depends on alleyinteractive/wp-type-extensions#29.

Copy link

@scottnelle scottnelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👻


Although it's possible to use XPath queries in conjunction with other `match_blocks()` arguments, the results with some arguments might be unexpected.

Typically, `match_blocks()` returns the blocks that match all the arguments. But when the `__experimental_xpath` argument is used, the set of source blocks will be first reduced to the blocks that match the XPath query, and then the remaining arguments will be applied.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might it be worth hinting this behavior in the name of the argument? Something like __experimental_xpath_block_pre_filter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't considered this approach, so I gave it some thought, but I decided to keep the existing name primarily because I don't want the name to become inaccurate in a future iteration.

It's not impossible to make the XPath argument work the same way as the other arguments, but it takes a bigger refactor than I feel like doing right now.

Also, practically speaking, I haven't yet imagined a situation where someone would use XPath in conjunction with the other affected arguments anyway. XPath can pretty much do everything the other arguments can, plus more.

@dlh01 dlh01 merged commit 2d3a2c8 into main Apr 9, 2025
2 checks passed
@dlh01 dlh01 deleted the feature/xpath branch April 9, 2025 01:51
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.

3 participants