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

Partial refactor to support substitutions in replacement strings as per spec #60

Closed

Conversation

throwanexception
Copy link

@throwanexception throwanexception commented Mar 30, 2018

This is a partial refactor to bring uap-python closer to the specification in uap-core.

It supports regular expression backreferences for replacements for all parsers - uap-core has started to use these for UAParser and OSParser instances, and only DeviceParser currently supports. (note: this was true of the regexes I imported at the time - it appears uap-core had done some cleanup and this is somewhat better now)

I believe the spec (https://github.com/ua-parser/uap-core/blob/master/docs/specification.md)
does indicate that all matches from $1 to $9 should be honored for every
replacement.

Each parser contains a list of regular-expressions which are named regex. For each regex replacements specific to the parser can be named to attribute or change information. A replacement may require a match from the regular-expression which is extracted by an expression enclosed in normal brackets "()". Each match can be addressed with $1 to $9 and used in a parser specific replacement.

It additionally allows for a v3 replacement for UserAgent.

Refactored the class hierarchy so that it's sort of DRY, and moved MultiReplace to a module level function because it doesn't use self.

Looking forward to any feedback!

@throwanexception throwanexception force-pushed the partial_refactor branch 3 times, most recently from 24f722b to 4ea8ff8 Compare March 30, 2018 05:54
@ondras
Copy link
Contributor

ondras commented Apr 5, 2018

I think I solved the very same issue (in my own branch) using a simpler way: 6539b05

replacements. uap-core has started to use these for UAParser and
OSParser instances, and only DeviceParser currently supports.

The spec (https://github.com/ua-parser/uap-core/blob/master/docs/specification.md)
indicates that all matches from $1 to $9 should be honored for every
replacement.

It also allows for a v3 replacement for UserAgent (which is used for at least one
useragent at this point in time), so I added that as well.

Also refactored the class hierarchy so that it's sort of DRY,
and moved MultiReplace to a module level fuction because it doesn't use
self.
@throwanexception
Copy link
Author

Thanks! I rebased and got your commit that fixes the issue, much better.

@throwanexception throwanexception changed the title Partial refactor + bump uap-core submodule Partial refactor to support substitutions in replacement strings as per spec Apr 11, 2018
@masklinn
Copy link
Contributor

masklinn commented May 2, 2022

Heya @throwanexception, it looks like the extraction and reuse of MultiReplace was independently performed in 7d61e38.

Assuming you still care, should I close this? v3 was never added to UserAgentParser, but there I would suggest getting a test case in uap-core for all implementations to leverage. Though I'm not sure uap-core currently has a "synthetic" test suite (which tests artificial cases against the spec rather than real-world parses), so that would probably require finding a real-world user agent which provides a use-case, as uap-python doesn't currently have one such test suite either (might be a good idea to add one as part of #102, unless -core is interested in adding one).

@masklinn masklinn closed this Aug 10, 2022
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