Opened 3 years ago
Closed 2 years ago
#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)
Change History (16)
comment:1 Changed 3 years ago by tscrim
- Description modified (diff)
comment:2 Changed 3 years ago by tscrim
- Status changed from new to needs_review
comment:3 Changed 3 years ago by vbraun
- Report Upstream changed from N/A to Reported upstream. No feedback yet.
- Status changed from needs_review to needs_info
I've asked on IPython-dev at http://python.6.x6.nabble.com/InputSplitter-and-SyntaxError-td5025938.html
comment:4 Changed 3 years ago by tscrim
Thanks Volker. I was going to do that first thing this morning (i.e. right now).
comment:5 Changed 3 years 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 2 years 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 2 years 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 2 years ago by vbraun
- Status changed from needs_review to positive_review
Looks good to me.
comment:9 Changed 2 years ago by jdemeyer
- Summary changed from Preparser not robust enough against typos: to Preparser not robust enough against typos
Changed 2 years ago by jdemeyer
Changed 2 years ago by jdemeyer
comment:10 Changed 2 years ago by jdemeyer
- Description modified (diff)
- Status changed from positive_review to needs_work
comment:11 Changed 2 years ago by jdemeyer
- Status changed from needs_work to needs_review
14961_doctest.patch needs review.
comment:12 Changed 2 years 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 2 years ago by vbraun
Followup at #15075 for a proper fix.
comment:14 Changed 2 years ago by jdemeyer
- Merged in set to sage-5.12.beta4
- Resolution set to fixed
- Status changed from positive_review to closed
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...