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

Updated to work around StrAndUnicode removal in 1.7 (fixes #61) #62

Merged
merged 4 commits into from
Dec 5, 2014
Merged

Updated to work around StrAndUnicode removal in 1.7 (fixes #61) #62

merged 4 commits into from
Dec 5, 2014

Conversation

robhudson
Copy link
Contributor

No description provided.

@robhudson robhudson mentioned this pull request Oct 8, 2014
@robhudson
Copy link
Contributor Author

r? @jsocol or @davedash ? Pretty please with a 🍒 on top?

@jsocol
Copy link
Collaborator

jsocol commented Oct 21, 2014

I will take a look this weekend. I'm sorry I can't get it sooner.

from django.utils.encoding import python_2_unicode_compatible

@python_2_unicode_compatible
class StrAndUnicode:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a new-style class

@jsocol
Copy link
Collaborator

jsocol commented Oct 21, 2014

I'm not 100% sure the monkeypatch is needed in 1.7, since Django started supporting the __html__ protocol. I need to dig into that. If we don't need it, we don't need to fix the monkeypatch, just document the change.

@robhudson
Copy link
Contributor Author

Updated based on comments.

I can play with zamboni under Django 1.7 and without the monkey patch. I agree, it looks like it isn't needed now that there's this.

@robhudson
Copy link
Contributor Author

Turns out the monkey patching is still needed. I found cases where forms rendered incorrectly without it. I didn't dig much further than that.

@jsocol
Copy link
Collaborator

jsocol commented Oct 21, 2014

Can you post the cases you found? I'll dig more this weekend but I want to understand why the code has to be there. 

Sent from my phone, please excuse the occasional typo.

On Tue, Oct 21, 2014 at 7:15 PM, Rob Hudson notifications@github.com
wrote:

Turns out the monkey patching is still needed. I found cases where forms rendered incorrectly without it. I didn't dig much further than that.

Reply to this email directly or view it on GitHub:
#62 (comment)

@robhudson
Copy link
Contributor Author

I simply commented out the monkey patch and loaded a view with a form in zamboni and saw a bunch of unrendered HTML for the form elements. I'd have to look at how SafeData is used in django to understand better what's going on.

@jsocol jsocol mentioned this pull request Oct 23, 2014
@robhudson
Copy link
Contributor Author

I'm wondering if we can get this merged?

@jsocol
Copy link
Collaborator

jsocol commented Dec 5, 2014

OK, I think I see what's happening: if StrAndUnicode isn't there, then only the last 3 lines matter because, for some reason, some classes there don't have the __html__ method.

I'm going to merge this as-is but eventually I'd like to not have the extra class with the runtime basis. Python is cray but that's a little far.

jsocol added a commit that referenced this pull request Dec 5, 2014
Updated to work around StrAndUnicode removal in 1.7 (fixes #61)
@jsocol jsocol merged commit fce0904 into jbalogh:master Dec 5, 2014
@jsocol jsocol mentioned this pull request May 8, 2015
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.

None yet

2 participants