Opened 10 years ago

Closed 10 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:

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 10 years ago.
Adds '# -*- coding: utf-8 -*-' to the preparsing code.
trac_7835-preparsing-unicode_v2.patch​ (2.1 KB) - added by mpatel 10 years ago.
Add short-term workaround for History/Log?. Replaces previous.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 10 years ago by timdumol

  • Cc was mpatel added
  • Status changed from new to needs_review

This should do the trick.

Changed 10 years ago by timdumol

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

comment:2 Changed 10 years ago by mpatel

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

comment:3 Changed 10 years ago by timdumol

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

print 'é'

comment:4 Changed 10 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 10 years ago by mpatel

Now reviewing #7249...

comment:6 Changed 10 years ago by mpatel

  • Cc ddrake added
  • Status changed from needs_review to needs_work

Changed 10 years ago by mpatel

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

comment:7 Changed 10 years ago by mpatel

  • Reviewers set to Mitesh Patel
  • Status changed from needs_work to 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 10 years ago by mpatel

  • Status changed from needs_review to positive_review

comment:9 Changed 10 years ago by mpatel

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

comment:10 Changed 10 years ago by timdumol

  • Merged in set to sagenb-0.6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.