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

don't break while generating tokens for names with non-ascii characters #23

Merged
merged 1 commit into from
Dec 2, 2015

Conversation

davisagli
Copy link
Member

(which could come from user input in the creator field,
see plone/plone.app.widgets#120)

@davisagli
Copy link
Member Author

@davisagli
Copy link
Member Author

@davisagli davisagli force-pushed the davisagli-fix-user-nonascii branch from 1d3dacb to e4c7d12 Compare November 28, 2015 06:56
@davisagli
Copy link
Member Author

Fixed test, trying again: http://jenkins.plone.org/job/pull-request-5.0/603/

@davisagli
Copy link
Member Author

Passing

@jensens
Copy link
Member

jensens commented Dec 1, 2015

fetching fullname can be expensive, did you try this with many users?

@davisagli
Copy link
Member Author

I didn't change that part, I just made it encode the value to create the token when it needs to, and reduced code duplication.

@thet
Copy link
Member

thet commented Dec 1, 2015

looks good, only i'm wondering why you are using the unicode_escape encoding for the token and not quoted-printable encoding, like for plone.app.vocabularies.catalog KeywordsVocabulary?

@jensens
Copy link
Member

jensens commented Dec 2, 2015

I'd just merge, i think which encoding is used here does not make a big difference. I'd have used quoted-printable too, since its an encoding which can be used outside python world too easily.
But we may want to change this in another PR if this is a thing.

jensens added a commit that referenced this pull request Dec 2, 2015
don't break while generating tokens for names with non-ascii characters
@jensens jensens merged commit 2d21323 into master Dec 2, 2015
@jensens jensens deleted the davisagli-fix-user-nonascii branch December 2, 2015 09:04
jensens added a commit to plone/buildout.coredev that referenced this pull request Dec 2, 2015
@thet
Copy link
Member

thet commented Dec 2, 2015

+1

@davisagli
Copy link
Member Author

I used unicode_escape out of habit...I wasn't really familiar with quoted-printable, but it looks like a good choice too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants