Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Add more members of filter_parser to milli:: & From<&str> implementation for Token #719

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Add more members of filter_parser to milli:: & From<&str> implementation for Token #719

merged 2 commits into from
Dec 6, 2022

Conversation

GregoryConrad
Copy link
Contributor

@GregoryConrad GregoryConrad commented Dec 3, 2022

What does this PR do?

The current milli::Filter and milli::FilterCondition APIs require working with some members of filter_parser directly that milli:: does not re-export to its users (at least when not parsing input using parse). Also, using filter_parser does not make sense when using milli from an embedded context where there is no query to parse.

Instead of reworking milli::Filter and milli::FilterCondition, this PR adds two non-breaking changes that ease the use of milli:

  • Re-exports more members of the dependent version of filter_parser in milli
  • Implements From<&str> for filter_parser::Token
    • This will also allow some basic tests that need to create a Token from a string to avoid some boilerplate.

In conjunction, both of these will allow milli users to easily create a Token from a &str without needing to add filter_parser as an extra dependency.

Note: I wanted to use FromStr for the From implementation; however, it requires returning a Result which is not needed for the conversion. Thus, I just left it as From<&str>.

@GregoryConrad GregoryConrad changed the title Add milli::filter_parser & From<&str> implementation for Token Add more members of filter_parser to milli:: & From<&str> implementation for Token Dec 5, 2022
@irevoire irevoire added the no breaking The related changes are not breaking (DB nor API) label Dec 5, 2022
Copy link
Member

@irevoire irevoire left a comment

Choose a reason for hiding this comment

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

That's good for me.
I'll let @Kerollmops merge if he's ok with the PR as well.

@irevoire irevoire requested a review from Kerollmops December 5, 2022 19:32
Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

Thank you very much. Looks good to me, merging!
bors merge

@bors
Copy link
Contributor

bors bot commented Dec 6, 2022

@bors bors bot merged commit 2a846aa into meilisearch:main Dec 6, 2022
@GregoryConrad GregoryConrad deleted the filter-parser-convenience branch January 12, 2023 19:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
no breaking The related changes are not breaking (DB nor API)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants