Opened 6 months ago
Closed 6 months ago
#16196 closed defect (fixed)
dedent pasted sage prompts
Reported by: | vbraun | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.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 kcrisman)
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 "<ipython-input-34-360e106e9b2f>", line 1 sage: Integer(1)+Integer(1) # indent + prompt = fail ^ SyntaxError: invalid syntax
Possibly due to #16050
Change History (12)
comment:1 Changed 6 months ago by kcrisman
- Description modified (diff)
comment:2 Changed 6 months ago by ohanar
comment:3 Changed 6 months ago by ohanar
- Branch set to u/ohanar/ge_prompt_dedents
comment:4 Changed 6 months ago by ohanar
- 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 follow-up: ↓ 6 Changed 6 months ago by 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 backward-compatibility?
- 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 6 months ago by ohanar
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 backward-compatibility?
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 re-break #16154).
comment:7 Changed 6 months ago by ohanar
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 6 months ago by jhpalmieri
- 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 6 months ago by git
- 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 6 months ago by jhpalmieri
- Status changed from needs_review to positive_review
comment:11 Changed 6 months ago by leif
Ah, ok... :-)
comment:12 Changed 6 months ago by vbraun
- 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.