Opened 13 years ago

Closed 13 years ago

#7835 closed defect (fixed)

Preparsing on server does not account for unicode text

Reported by: timdumol Owned by: was
Priority: major Milestone: sage-4.3.1
Component: notebook Keywords:
Cc: was, mpatel, ddrake Merged in: sagenb-0.6
Authors: Tim Dumol Reviewers: Mitesh Patel
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

#7483 moves preparsing to the server but does not account for unicode text, i.e., does not have a # -*- coding: utf-8 -*- header.

This patch depends on #7514 and everything it depends on.

Attachments (2)

trac_7835-preparsing-unicode.patch​ (856 bytes) - added by timdumol 13 years ago.
Adds '# -*- coding: utf-8 -*-' to the preparsing code.
trac_7835-preparsing-unicode_v2.patch​ (2.1 KB) - added by mpatel 13 years ago.
Add short-term workaround for History/Log?. Replaces previous.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 13 years ago by timdumol

Cc: was mpatel added
Status: new → needs_review

This should do the trick.

Changed 13 years ago by timdumol

Attachment: trac_7835-preparsing-unicode.patch​ added

Adds '# -*- coding: utf-8 -*-' to the preparsing code.

comment:2 Changed 13 years ago by mpatel

This looks good to me. Is there a simple example that breaks the existing code?

comment:3 Changed 13 years ago by timdumol

You mean without this patch? Just use any non-ASCII character and attempt to evalaute it. 

print 'é'

comment:4 Changed 13 years ago by mpatel

Just to check: I should apply #7249, too. Otherwise, even with this patch, print 'é' raises 

          File "/home/sage/notebook/sagenb-0.4.9/sagenb/notebook/twist.py", line 1205, in render
            max_out=HISTORY_MAX_OUTPUT)
        exceptions.UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 17: ordinal not in range(128)

at least for me.

comment:5 Changed 13 years ago by mpatel

Now reviewing #7249...

comment:6 Changed 13 years ago by mpatel

Cc: ddrake added
Status: needs_review → needs_work

Changed 13 years ago by mpatel

Attachment: trac_7835-preparsing-unicode_v2.patch​ added

Add short-term workaround for History/Log?. Replaces previous.

comment:7 Changed 13 years ago by mpatel

Reviewers: → Mitesh Patel
Status: needs_work → needs_review

V2 wraps the problem above in a try-except block, for now, i.e., until #7249 takes hold. My review is positive, but someone should review my change.

comment:8 Changed 13 years ago by mpatel

Status: needs_review → positive_review

comment:9 Changed 13 years ago by mpatel

I'm assuming my change is OK, given its position, but feel free to revert the status.

comment:10 Changed 13 years ago by timdumol

Merged in: → sagenb-0.6
Resolution: → fixed
Status: positive_review → closed
Note: See TracTickets for help on using tickets.