ipython's reaction to ctrl-c when reverse-searching history #1286

Closed
segasai opened this Issue Jan 18, 2012 · 7 comments

Comments

Projects
None yet
5 participants
@segasai

segasai commented Jan 18, 2012

Hi,

I've noticed a relatively minor bug with reverse searching the history in ipython, but it can be quite annoying.

In [1]: print 'aaaa'
aaaa

In [2]:  # Now pressing Ctrl-r  to search for aaa
(reverse-i-search)`aa': print 'aaaa'  
#Now I cancel the search by pressing Ctrl-C
KeyboardInterrupt

In [2]:     

So what I get is the empty command line -- which is perfectly fine, but the problem if I press enter now the command from the history still is going to be executed...
I checked and this definitely doesn't happen in the same situation with e.g. bash's readline.

Sergey

kanzure added a commit to kanzure/ipython that referenced this issue Feb 7, 2013

better reverse search KeyboardInterrupt handling
The previous KeyboardInterrupt handling is dangerous because previous
lines or commands can be mistakenly executed.

When a user interrupts a readline reverse search (C-r) with C-c, a
KeyboardInterrupt is thrown, but the readline buffer still has a line in
the buffer from the search attempt. The previous behavior of IPython was
to show a blank line instead of the current readline buffer, causing a
previous command to be executed if the user presses enter on the
seemingly blank line.

Additionally, keyboard input was not shown on the blank line because
readline was still in reverse search mode. For example, pressing C-k on
the blank line would reset readline, but most users would expect the
input buffer that IPython displays to be correct because of the
prominently displayed KeyboardInterrupt.

This modifies how a KeyboardInterrupt is handled in IPython. Under
normal circumstances in the operation of readline, a KeyboardInterrupt
should never be thrown. IPython now displays a helpful reminder to use
default readline keybindings (which might be overridden by ~/.inputrc,
but whatever), and then displays the current line buffer on the next
line.

Another possible solution would be to send C-g (abort reverse search) or
C-k to readline through raw_input somehow. However, this solution would
require checking ~/.inputrc to figure out what the current "abort
reverse search" keybinding is.

fixes #1286
fixes #2486
@kanzure

This comment has been minimized.

Show comment Hide comment
@kanzure

kanzure Feb 7, 2013

I think readline expects you to use C-g (ctrl+g) to get out of reverse searching mode. There should be no reason to use C-c (ctrl+c) in the normal operation of ipython or readline because you have the default keybindings that you can override in your ~/.inputrc file.

This also seems to be related to the following upstream issues:

I have also submitted pull request #2895 to deal with #1286 and #2486.

kanzure commented Feb 7, 2013

I think readline expects you to use C-g (ctrl+g) to get out of reverse searching mode. There should be no reason to use C-c (ctrl+c) in the normal operation of ipython or readline because you have the default keybindings that you can override in your ~/.inputrc file.

This also seems to be related to the following upstream issues:

I have also submitted pull request #2895 to deal with #1286 and #2486.

@kanzure

This comment has been minimized.

Show comment Hide comment
@kanzure

kanzure Feb 8, 2013

Also, I feel I should elaborate that I think the current situation is very dangerous. It's dangerous because commands can be re-executed in different contexts while writing code, possibly with different values that can compromise a project, file system, database, etc.

My proposed patch causes all KeyboardInterrupts to be intercepted (not just those caused during reverse search, which I couldn't figure out how to detect separately anyway). However, you shouldn't be causing a KeyboardInterrupt during normal use of ipython anyway (such as searching, writing, editing, deleting), so I think the proposed change is not too intrusive

I should note that the proposed patch causes C-c on a normal line to cause the same line to appear in the input after recovering from the KeyboardInterrupt, but the user is given a message explaining the other more relevant keybindings anyway so the damage is minimal. Maybe there's a better way to solve this. For now, I think the best option is to immediately get rid of the dangerous situation with my patch.

kanzure commented Feb 8, 2013

Also, I feel I should elaborate that I think the current situation is very dangerous. It's dangerous because commands can be re-executed in different contexts while writing code, possibly with different values that can compromise a project, file system, database, etc.

My proposed patch causes all KeyboardInterrupts to be intercepted (not just those caused during reverse search, which I couldn't figure out how to detect separately anyway). However, you shouldn't be causing a KeyboardInterrupt during normal use of ipython anyway (such as searching, writing, editing, deleting), so I think the proposed change is not too intrusive

I should note that the proposed patch causes C-c on a normal line to cause the same line to appear in the input after recovering from the KeyboardInterrupt, but the user is given a message explaining the other more relevant keybindings anyway so the damage is minimal. Maybe there's a better way to solve this. For now, I think the best option is to immediately get rid of the dangerous situation with my patch.

@bfroehle

This comment has been minimized.

Show comment Hide comment
@bfroehle

bfroehle Feb 8, 2013

Contributor

Yes, I agree. I've taken to hitting ctrl-c twice to work around this.

Contributor

bfroehle commented Feb 8, 2013

Yes, I agree. I've taken to hitting ctrl-c twice to work around this.

@minrk

This comment has been minimized.

Show comment Hide comment
@minrk

minrk Mar 29, 2013

Owner

PR #2895 has a partial implementation of this fix, but has been unable to get back the proper behavior when not in reverse-search mode. I would recommend it as a starting point for anyone who wants to tackle this.

Owner

minrk commented Mar 29, 2013

PR #2895 has a partial implementation of this fix, but has been unable to get back the proper behavior when not in reverse-search mode. I would recommend it as a starting point for anyone who wants to tackle this.

minrk added a commit to minrk/ipython that referenced this issue Mar 30, 2013

prefer pyrepl to readline
avoids various readline-related issues, such as #1286.

By preferring pyrepl to readline, we can remove the gross sys.path hack to get around System lib edit readline on OS X.

@minrk minrk removed the prio-medium label Jan 14, 2015

@tacaswell tacaswell referenced this issue in NSLS-II/bluesky Jan 30, 2016

Closed

reverse search and cancel issue #309

@tacaswell

This comment has been minimized.

Show comment Hide comment
@tacaswell

tacaswell Feb 1, 2016

Contributor

Also see https://bugs.python.org/issue24266 which has a patch to fix this behavior.

Contributor

tacaswell commented Feb 1, 2016

Also see https://bugs.python.org/issue24266 which has a patch to fix this behavior.

@tacaswell

This comment has been minimized.

Show comment Hide comment
@tacaswell

tacaswell Feb 1, 2016

Contributor

Also see ludwigschwardt/python-gnureadline#47

Building that branch will give you a gnureadline (which IPython prefers) which correctly deals with this case.

Contributor

tacaswell commented Feb 1, 2016

Also see ludwigschwardt/python-gnureadline#47

Building that branch will give you a gnureadline (which IPython prefers) which correctly deals with this case.

@minrk minrk modified the milestones: not ipython, wishlist Feb 1, 2016

@minrk

This comment has been minimized.

Show comment Hide comment
@minrk

minrk Feb 1, 2016

Owner

@tacaswell that's super great. Closing here as the fix is on its way upstream.

Owner

minrk commented Feb 1, 2016

@tacaswell that's super great. Closing here as the fix is on its way upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment