Code

Opened 7 years ago

Closed 7 years ago

#4161 closed (fixed)

[unicode] Convert oldforms

Reported by: Ivan Sagalaev <Maniac@…> Owned by: mtredinnick
Component: Uncategorized Version: other branch
Severity: Keywords: unicode
Cc: Maniac@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Oldforms are now broken since they rely heavily on explicit str()s all over the place. I have a fixed version in my local tree and will make a patch after #4152 is committed.

Attachments (1)

4161.diff (15.5 KB) - added by Ivan Sagalaev <Maniac@…> 7 years ago.
Patch

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by mtredinnick

  • Cc mtredinnick removed
  • Needs documentation unset
  • Needs tests unset
  • Owner changed from jacob to mtredinnick
  • Patch needs improvement unset

Changed 7 years ago by Ivan Sagalaev <Maniac@…>

Patch

comment:2 Changed 7 years ago by Ivan Sagalaev <Maniac@…>

  • Has patch set

This fixes displaying fields in the current admin.

Updating objects still doesn't work but it's another issue. Basically I think we should wait for newforms-admin to merge. Otherwise we'll have to chase all str()'s in the current admin.

comment:3 Changed 7 years ago by mtredinnick

Yeah, I don't know what to do about "existing admin". Actually, I think I'm going to have to convert it, because we'll be ready first (and the existing i18n string problems are hurting people). However, I am going to leave it until the absolutely last thing to do so that there's still a chance it won't need to be done.

Patch itself looks good. In django/oldforms/__init__.py, is the explicit use of

unicode(self).encode('utf-8')

rather than smart_string(self) because you like messing with my head or for a good technical reason? I don't really mind -- so don't bother changing it -- since I suspect there's no difference here, but the fact that you are writing it this way makes me think you are doing it for a reason.

comment:4 Changed 7 years ago by Ivan Sagalaev <Maniac@…>

This explicit unicode(self) is because this same class has __unicode__ defined so we don't need smart_str to check if it's there or not. Besides wouldn't calling smart_str inside of __str__ lead to an infinite recursion?

comment:5 Changed 7 years ago by mtredinnick

Yep, good point about the recursion. I've walked into that before, too. Apparently didn't learn my lesson. :-)

comment:6 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [5090]) unicode: Fixed #4161 -- Ported oldforms internal string handling across to use
unicode. Still some breakage in admin, but that is caused by other problems.
Another contribution from Ivan Sagalaev.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.