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

Update to regex-automata v0.4 #5

Merged
merged 1 commit into from
Jul 1, 2024
Merged

Conversation

Mark-Simulacrum
Copy link
Contributor

This updates to use regex-automata v0.4, bumping the crate to 0.2 since regex-automata is a public dependency of the crate. All tests in this crate continue to pass.

This removes the ToMatcher trait in favor of directly using the upstream Automaton trait, it didn't seem like that trait was really being used. As of the current PR, there's no way to construct Patterns with the sparse DFA (this seems to have been true of the prior version of the crate too), but all the functionality is generic over regex_automata::Automaton, so just adding constructs for the lazy DFA
would be enough to make it possible to use.

This also reduces the enabled features on regex-automata to not include unicode features. It's not obvious that downstream crates (in particular tracing-subscriber) want to pay the binary size cost for having those enabled in env-filter regexes, and this leaves the choice up to them.

Resolves #3.

@plugwash
Copy link

Are you aware of the previous pull request addeessing the same issue and the discussion there?

@Mark-Simulacrum
Copy link
Contributor Author

I missed the last comments on it that it was on the latest version as well, which is why I started working on this, but yes I'm broadly aware.

@uduerholz
Copy link

@hawkw: Any chance, this commit will get merged and the crate published on crates.io? This would be great.

@extrawurst
Copy link

This would be lovely to see progress

@teohhanhui teohhanhui mentioned this pull request May 7, 2024
4 tasks
Copy link
Owner

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Given that v0.4 of regex-automata actually allows constructing an automaton in a start state without providing the input, unlike v0.3, I think this update is viable. Thanks for working on this, I'll be happy to merge it.

I did leave some comments on potential changes you may want to make, but they're not blockers for moving forwards with this change.

src/lib.rs Outdated
Comment on lines 144 to 148
let config = regex_automata::util::start::Config::new().anchored(if self.anchored {
Anchored::Yes
} else {
Anchored::No
});
Copy link
Owner

Choose a reason for hiding this comment

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

take it or leave it: why not just replace the anchored: bool field on Pattern with an Anchored enum value, and do this when the Pattern is constructed, instead?

Cargo.toml Show resolved Hide resolved
@Mark-Simulacrum
Copy link
Contributor Author

@hawkw I think both comments are fixed now.

@erickt
Copy link

erickt commented Jun 20, 2024

Gentle ping on if we could review this. One nice benefit for this is that regex-automata changed license from being MIT+Unlicense to MIT+Apache 2.0 in version 0.3.0, which might avoid licensing issues for some users.

@erickt
Copy link

erickt commented Jul 1, 2024

Thanks so much! Really appreciate it.

@jayvdb jayvdb mentioned this pull request Jul 2, 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.

Update regex-automata dependency
6 participants