-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
only strip continuation prompts if regular prompts seen first #4060
Conversation
should be discussed before merge (ping @takluyver), as it is a significant and sensitive change. I'm also not sure that it's the right fix, but worth a try. closes ipython#4059
return _strip_prompts(prompt_re) | ||
prompt_re = re.compile(r'^(In \[\d+\]: )') | ||
continuation_re = re.compile(r'^(In \[\d+\]: |\ \ \ \.\.\.+: )') | ||
return _strip_prompts(prompt_re, continuation_re) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ipy prompt does seem to be unambiguous as they avoid not to end with :
and that will help in case like when you can select the first line without leaving the first prompt not unselected. So would you like to call return _strip_prompts(continuation_re, continuation_re)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might be preferable. I'd like feedback from @takluyver on whether this seems like the right approach.
Keeping the existing behavior just means making the same call, so I can leave ipy_prompt
unchanged if that's the way we want to go.
I think this is closer to what we were doing before, but we changed it because quite a few people, faced with an example like this:
will copy from immediately after the initial prompt, so we get something like this pasted:
I think this case is rather more common than having the ellipsis singleton as the first element on the second line of the block (we already leave it if there isn't something that looks like a prompt in the first two lines). We could make it slightly more specific by checking the first line only for the initial |
- initial-only prompt is only stripped from the first line (only change from master) - any prompt is stripped from the second line - if a prompt is found in the first two lines, strip from the rest of the block
Logic updated after discussion with @takluyver:
Mostly the logic is unchanged - the only change being the addition of the smaller There are still some cases that will fail, which I think is inevitable given the ambiguity. But I think the affected cases are rare enough and easy enough to workaround that this is at least an improvement over master:
workaround for 1. is to use initial prompt or no prompt on the first line. |
👍 I agree with the behaviour, and the implementation looks good. We'll give it a day or two in case anyone wants to discuss the behaviour more. |
This seems like as good of a workaround as I can think of. You can probably be smarter in the qtconsole, but that's your decision if you want to implement the different logic there. |
only strip continuation prompts if regular prompts seen first
… seen first should be discussed before merge (ping @takluyver), as it is a significant and sensitive change. I'm also not sure that it's the right fix, but worth a try. closes #4059
only strip continuation prompts if regular prompts seen first
should be discussed before merge (ping @takluyver), as it is a significant and sensitive change.
I'm also not sure that it's the right fix, but worth a try.
closes #4059