Opened 3 years ago
Closed 3 years ago
#16196 closed defect (fixed)
dedent pasted sage prompts
Reported by:  vbraun  Owned by:  

Priority:  major  Milestone:  sage6.2 
Component:  misc  Keywords:  
Cc:  ohanar  Merged in:  
Authors:  R. Andrew Ohana  Reviewers:  John Palmieri 
Report Upstream:  N/A  Work issues:  
Branch:  101a6be (Commits)  Commit:  101a6beaae0109dffad9a8d93f8ebd44843794e5 
Dependencies:  Stopgaps: 
Description (last modified by )
Sage prompts with leading whitespace are not removed correctly:
sage: 1+1 # works 2 sage: sage: 1+1 # works 2 sage: sage: 1+1 # indent + prompt = fail File "<ipythoninput34360e106e9b2f>", line 1 sage: Integer(1)+Integer(1) # indent + prompt = fail ^ SyntaxError: invalid syntax
Possibly due to #16050
Change History (12)
comment:1 Changed 3 years ago by
 Description modified (diff)
comment:2 Changed 3 years ago by
comment:3 Changed 3 years ago by
 Branch set to u/ohanar/ge_prompt_dedents
comment:4 Changed 3 years ago by
 Commit set to a1354669efb14821dda9c9c4c5d9361dc7d08bb2
 Status changed from new to needs_review
This should work for now, although I looked a bit into the upstream code, and that should probably be changed to be more configurable (since after all the prompt is configurable).
New commits:
a135466  fix to dedent pasted sage prompts

comment:5 followup: ↓ 6 Changed 3 years ago by
Oh, sorry, I didn't see this ticket and introduced #16232 which strips leading whitespace. Do we close that one as a duplicate? (Meanwhile, it has a positive review, by the way.)
 Why only strip one prompt? I see the explanation that we are trying to match IPython's behavior, but do we need to preserve backwardcompatibility?
 What is the point of the change to
src/sage/repl/ipython_extension.py
? Just changing the regular expression (as at #16232) seems to solve the problem.
comment:6 in reply to: ↑ 5 Changed 3 years ago by
Replying to jhpalmieri:
Oh, sorry, I didn't see this ticket and introduced #16232 which strips leading whitespace. Do we close that one as a duplicate? (Meanwhile, it has a positive review, by the way.)
 Why only strip one prompt? I see the explanation that we are trying to match IPython's behavior, but do we need to preserve backwardcompatibility?
Is there sage code that is being written with sage (duplicate) prompts? Really duplicate prompts should only come about by copying something into a terminal session, and then copying that pasted content again  this seems like a complete edge case that wouldn't (and certainly shouldn't) show up in some code that is meant to be used for a long period of time.
 What is the point of the change to
src/sage/repl/ipython_extension.py
? Just changing the regular expression (as at #16232) seems to solve the problem.
IPython already has a dedenter, and it is smart (it strips an equal amount from each line), so all we need to do is strip our prompts after ipython dedents, and for the moment, that means insert after position 0 (and before position 1, so as to not rebreak #16154).
comment:7 Changed 3 years ago by
Like I mentioned, this should just be configurable with IPython itself  we shouldn't have to be pulling this internal _strip_prompts
function in the first place. I'll make a patch if I ever find the time to fix this upstream, but I'm much to busy at the moment.
comment:8 Changed 3 years ago by
 Branch changed from u/ohanar/ge_prompt_dedents to u/jhpalmieri/dedent
 Commit changed from a1354669efb14821dda9c9c4c5d9361dc7d08bb2 to fc520709f1682205ac55f7c75f31eb84d9781587
 Reviewers set to John Palmieri
 Status changed from needs_review to positive_review
Rebased (w.r.t. #16232).
comment:9 Changed 3 years ago by
 Commit changed from fc520709f1682205ac55f7c75f31eb84d9781587 to 101a6beaae0109dffad9a8d93f8ebd44843794e5
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
101a6be  #16196: rebased to #16232

comment:10 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:11 Changed 3 years ago by
Ah, ok... :)
comment:12 Changed 3 years ago by
 Branch changed from u/jhpalmieri/dedent to 101a6beaae0109dffad9a8d93f8ebd44843794e5
 Resolution set to fixed
 Status changed from positive_review to closed
It looks like the change from #16154 broke this (ipython's leading indent stripper is now being run after our prompt stripper). I can look into this sometime later this week.