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

Use/depend on user-agents package for better analysis of user agent strings? #11

Closed
wodow opened this issue Aug 21, 2014 · 6 comments
Closed

Comments

@wodow
Copy link

wodow commented Aug 21, 2014

I've recently been using Flask-Mobility for a project or two (so thank you!) and quickly reached the point where I (a) needed more power, but (b) wanted to keep using the Flask-Mobility API/interface.

What do you think about using user-agents ( https://github.com/selwin/python-user-agents/ ) to support finer-grained and more accurate analysis if UAs, in place of the current regex approach?

I've just forked the repo to create a proof-of-concept: wodow@6b92262#diff-9da72ae617e53ab1abe6152bb885ed15

@rehandalal
Copy link
Owner

sounds like an interesting idea. just a couple of thoughts:

  • I don't want to go overboard littering the request object with new attributes. Maybe there's a cleaner way of implementing what's there.
  • Your fork removes the ability to force the mobile view using a cookie. I don't want to lose that.
  • Would likely want to create new decorators or extend the functionality of the current decorators to accommodate some of the new features.

also feel free to submit a pull request so that it's easier to compare the changes.

@wodow
Copy link
Author

wodow commented Aug 22, 2014

Thanks for the fast reply!

I should have been clearer about the removal of the cookies code (and the lack of a PR) - I did it to simplify the proof-of-concept (which I have running locally).

I am more than happy to submit a proper, cleanly edited PR with additional decorators if you are interested.

My main question is: are you happy to have a dependency on user-agent? Or should it be optional somehow?

Re the new attributes on the request object: I considered introducing a second level of access, e.g. request.MOBILITY.TABLET or request.MOBILITY.is_mobile but this would not be backwards-compatible with existing deployments using request.MOBILE.

Is that an issue? A compromise would be to introduce something like request.MOBILITY but retain request.MOBILE (which would be aliased to e.g. request.MOBILITY.is_mobile).

@rehandalal
Copy link
Owner

Sorry about the delay getting back to you.

I'm fine with making user-agent a dependency. It would be nice to have it be optional, but that's not a deal breaker for me.

I would like to ensure this stays backwards compatible so I like the idea of having request.MOBILE and request.MOBILITY.

Thanks!

@wodow
Copy link
Author

wodow commented Aug 26, 2014

Thanks for the thumbs up. I will aim to pull something when I next make a related change in the client webapp, so can't be sure of a timeline just yet.

I will try to make user-agent optional as a secondary goal.

@helielson
Copy link

I thought about make the same thing using the user-agent lib.
It's really an interesting idea.
What about the PR @wodow? Are you still think to integrate it?
Let me know if you are needing some help.

@rehandalal
Copy link
Owner

@helielson i might get around to adding this in the next few weeks

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

No branches or pull requests

3 participants