#14961 closed defect (fixed)

Preparser not robust enough against typos

Reported by: tscrim Owned by: tscrim
Priority: critical Milestone: sage-5.12
Component: user interface Keywords:
Cc: Merged in: sage-5.12.beta4
Authors: Travis Scrimshaw, Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: Reported upstream. Developers acknowledge bug. Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

I made a typo and sage crashed:

R.<t> = QQ{]

Note that there is one opening squigly bracket { and one closing square bracket ].

Here's the backtrace

Traceback (most recent call last):
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/IPython/core/application.py", line 175, in excepthook
    return self.crash_handler(etype, evalue, tb)
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/IPython/core/crashhandler.py", line 178, in __call__
    raw_input("Hit <Enter> to quit (your terminal may close):")
  File "c_lib.pyx", line 70, in sage.ext.c_lib.sage_python_check_interrupt (sage/ext/c_lib.c:925)
KeyboardInterrupt

Original exception was:
Traceback (most recent call last):
  File "/home/tscrim/sage-5.11.beta3/local/bin/sage-ipython", line 18, in <module>
    app.start()
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/IPython/frontend/terminal/ipapp.py", line 363, in start
    self.shell.mainloop()
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/IPython/frontend/terminal/interactiveshell.py", line 467, in mainloop
    self.interact(display_banner=display_banner)
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/IPython/frontend/terminal/interactiveshell.py", line 579, in interact
    self.input_splitter.push(line)
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/misc/sage_extension.py", line 393, in push
    line = f(line, line_number)
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/misc/interpreter.py", line 271, in __call__
    return preparse(line, reset=(line_number==0))
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/misc/preparser.py", line 1112, in preparse
    L = preparse_generators(L)
  File "/home/tscrim/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/misc/preparser.py", line 977, in preparse_generators
    opening = constructor.rindex('[')
ValueError: substring not found

Apply [trac_14961-hack_fix_preparser-ts.patch​] and 14961_doctest.patch

Attachments (2)

trac_14961-hack_fix_preparser-ts.patch (2.4 KB) - added by jdemeyer 15 months ago.
14961_doctest.patch (1.2 KB) - added by jdemeyer 15 months ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 16 months ago by tscrim

  • Description modified (diff)

comment:2 Changed 16 months ago by tscrim

  • Authors set to Travis Scrimshaw
  • Status changed from new to needs_review

Here's a patch which is just a hack fix so that sage doesn't crash and it tells you that an error has occurred. It prints the error twice, but it works...

sage: R.<t> = QQ{]
SyntaxError: Mismatched ']'
SyntaxError: Mismatched ']'
sage:

comment:3 Changed 16 months ago by vbraun

  • Report Upstream changed from N/A to Reported upstream. No feedback yet.
  • Status changed from needs_review to needs_info

comment:4 Changed 16 months ago by tscrim

Thanks Volker. I was going to do that first thing this morning (i.e. right now).

comment:5 Changed 16 months ago by jdemeyer

  • Component changed from interfaces to user interface
  • Milestone changed from sage-5.11 to sage-5.12
  • Priority changed from blocker to critical

comment:6 Changed 16 months ago by vbraun

  • Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.
  • Reviewers set to Volker Braun
  • Status changed from needs_info to needs_work

The IPython devs acknowledged that this currently does not work. It sounds like they would accept a patch but don't plan on fixing this in the near future themselves. So fixing this properly will take some time.

In the meantime, the proposed workaround looks good. However, it has an unrelated change to in sage/algebras/lie_algebras/kac_moody.py that needs to be removed.

comment:7 Changed 15 months ago by tscrim

  • Status changed from needs_work to needs_review

Eeek, how did I let that get in there. Here's the patch without that change.

comment:8 Changed 15 months ago by vbraun

  • Status changed from needs_review to positive_review

Looks good to me.

comment:9 Changed 15 months ago by jdemeyer

  • Summary changed from Preparser not robust enough against typos: to Preparser not robust enough against typos

Changed 15 months ago by jdemeyer

Changed 15 months ago by jdemeyer

comment:10 Changed 15 months ago by jdemeyer

  • Authors changed from Travis Scrimshaw to Travis Scrimshaw, Jeroen Demeyer
  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:11 Changed 15 months ago by jdemeyer

  • Status changed from needs_work to needs_review

14961_doctest.patch needs review.

comment:12 Changed 15 months ago by vbraun

  • Status changed from needs_review to positive_review

Fine with me, though I think the effort should go in pushing this upstream not doctesting the temporary workaround.

comment:13 Changed 15 months ago by vbraun

Followup at #15075 for a proper fix.

comment:14 Changed 15 months ago by jdemeyer

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