-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/guessing language via accept language header #184
base: develop
Are you sure you want to change the base?
Feature/guessing language via accept language header #184
Conversation
Hey again, I would really like to push it into the 2.5 branch. It's not the most efficient code yet, but it's the most accurate one right now for guessing the preferred language a client would like to see. The X |
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.
I'm fine with this pull request, please change the two obsolete lines and move the definition of langSet into the else-branch.
__init__.py
Outdated
if len( tmppath )>0 and tmppath[0] in conf["viur.availableLanguages"]+list( conf["viur.languageAliasMap"].keys() ): | ||
self.language = tmppath[0] | ||
return( path[ len( tmppath[0])+1: ] ) #Return the path stripped by its language segment | ||
langSet = True |
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.
This line is obsole: You're setting langSet to True and return from the function afterwards.
__init__.py
Outdated
@@ -360,18 +360,40 @@ def selectLanguage( self, path ): | |||
elif conf["viur.languageMethod"] == "url": | |||
tmppath = urlparse.urlparse( path ).path | |||
tmppath = [ urlparse.unquote( x ) for x in tmppath.lower().strip("/").split("/") ] | |||
langSet = False |
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.
This line can be moved into the else part.
__init__.py
Outdated
else: # This URL doesnt contain an language prefix, try to read it from session | ||
if session.current.getLanguage(): | ||
self.language = session.current.getLanguage() | ||
elif "X-Appengine-Country" in self.request.headers.keys(): | ||
langSet = True | ||
if not langSet and "Accept-Language" in self.request.headers: |
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.
The control-flow is botched. Instead of carrying the marker langSet, just return path in L371. Same thing at the end of this if block - if found return path.
__init__.py
Outdated
if acceptLangHeader: | ||
acceptLangHeader = acceptLangHeader.split(",") | ||
for possibleLang in acceptLangHeader[:7]: | ||
try: |
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.
No need for try/except here. As you are not interested in regionEtc just call lng = possibleLang.split("-")[0] - which will always work
In a follow up PR I would also like reformat the complete file - it is still not using our viur coding style... |
Correctly call bound PeriodicTask functions
__init__.py
Outdated
self.language = lng | ||
return path | ||
elif ";" in lng: | ||
lng, weight = lng.split(";") |
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.
Just one more small thing. This Line can also fail. And you don't use weight at all. So also use lng.split(";")[0] - which doesn't break if there are more than two colons in that string
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.
Please add an entry in the CHANGELOG.md
With this patch it would also be possible to guess the users preferred language via Accept-Language client header.