-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Browser enums #62
Browser enums #62
Conversation
7c648b4
to
9d2a495
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few seemingly unrelated commits with nixos things in the beginning, which I ignored. The rest looks good, but I only did a visual check. I didn't test the actual code (yet).
Thanks for looking
Yeah, old habits die hard. |
827f967
to
91610ec
Compare
This helps provide a single entry and exit point for library users to ensure that any strings passed in are parsed into exactly one browser type. Will implement for Firefox shortly. This is partly in preparation for adding a CLI script, at which point this should be the only part of the app that accepts a user-provided string for a browser name; all other parts of the API should be passing an instance of `BrowserType`.
See also: 149a7fa
77347e2
to
6403b16
Compare
Enums seem like a more correct way to provide discrete browser variants, and since they're easily available in Python for many versions now, I wanted to try passing an enum between functions internally instead of a string. This should provide much better type checking during development. As an additional advantage, any time it is necessary to accept a string from a user, this should be able to consolidate the parsing logic to a single location.
As StrEnum wasn't available until relatively late in the game, I used the "inherit from
str, Enum
" trick to allow easy comparisons:... though I don't think I'm leveraging this anywhere so a straight
Enum
withauto()
variants would likely have been fine.Added a deprecation warning for users passing in strings; it should be minimal friction to adapt code from
browser="chrome"
tobrowser=BrowserType("chrome")
orbrowser=BrowserType.CHROME
.This is partly in preparation for adding a CLI tool, which I hope to embark on soon (#60), and I intend to finally add some proper logging as well (#46)
Would appreciate any thoughts if you have time @grandchild!