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

Rename the string regex attribute of matchers to regex #213

Merged
merged 1 commit into from
Jul 13, 2024

Conversation

masklinn
Copy link
Contributor

It's kinda dumb that I didn't do that in the first place and I'm not entirely sure why I missed it...

Anyway there is no reason to make pattern be the string pattern and regex be the compiled pattern, that unnecessarily diverges from the regexes.yaml naming for the corresponding attribute which is a shame, and the compiled regex in python is... re.Pattern, so it makes more sense on both axis to have pattern: re.Pattern[str] and regex: str.

Also add a regex_flag attribute/property on the device matchers for the string version of the flags.

With that, maybe we could build the matchers straight from the source records by direct unpacking, as well as simplify the codegen through the magic of doing less? This probably requires benching to see if

 foo(**{'foo': 'bar'})

has the same efficiency as

foo(foo=bar)

if it does, then there's no reason to bother with reformatting parser records to the current

foo(bar)

@masklinn masklinn changed the title Have the names of matcher attributes match regexes.yaml Rename the string regex attribute of matchers to regex Jul 13, 2024
@masklinn masklinn enabled auto-merge (rebase) July 13, 2024 13:19
It's kinda dumb that I didn't do that in the first place and I'm not
entirely sure why I missed it...

Anyway there is no reason to make `pattern` be the string pattern and
`regex` be the compiled pattern, that unnecessarily diverges from the
regexes.yaml naming for the corresponding attribute which is a shame,
and the compiled regex in python is... `re.Pattern`, so it makes more
sense on both axis to have `pattern: re.Pattern[str]` and `regex:
str`.

Also add a `regex_flag` attribute/property on the device matchers for
the string version of the flags.
@masklinn masklinn merged commit 4d988a0 into ua-parser:master Jul 13, 2024
29 checks passed
@masklinn masklinn deleted the match-regexes-names branch July 13, 2024 13:38
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