Permalink
Browse files

Merge pull request #929 from juliantaylor/multiline-history

some readline multiline-history fixes and tests

* fixes crash when readline is absent (#911)
* do not save input from e.g. raw_input in history
* do not add empty or duplicate entries lines to the history.
* add tests for _replace_rlhist_multiline
* protect pyreadline from some incorrect assumptions of equivalence to regular readline

closes gh-911
  • Loading branch information...
2 parents 2c5b840 + 2590197 commit 7c0ffa59eac01ba6f816c6baec0c788257f2e4c6 @minrk minrk committed Nov 7, 2011
@@ -308,7 +308,7 @@ def _exiter_default(self):
"""
)
multiline_history = CBool(sys.platform != 'win32', config=True,
- help="Store multiple line spanning cells as a single entry in history."
+ help="Save multi-line entries as one entry in readline history"
)
prompt_in1 = Unicode('In [\\#]: ', config=True)
@@ -231,13 +231,30 @@ def mainloop(self, display_banner=None):
def _replace_rlhist_multiline(self, source_raw, hlen_before_cell):
"""Store multiple lines as a single entry in history"""
- if self.multiline_history and self.has_readline:
- hlen = self.readline.get_current_history_length()
- for i in range(hlen - hlen_before_cell):
- self.readline.remove_history_item(hlen - i - 1)
- stdin_encoding = sys.stdin.encoding or "utf-8"
- self.readline.add_history(py3compat.unicode_to_str(source_raw.rstrip(),
- stdin_encoding))
+
+ # do nothing without readline or disabled multiline
+ if not self.has_readline or not self.multiline_history:
+ return hlen_before_cell
+
+ # windows rl has no remove_history_item
+ if not hasattr(self.readline, "remove_history_item"):
+ return hlen_before_cell
+
+ # skip empty cells
+ if not source_raw.rstrip():
+ return hlen_before_cell
+
+ # nothing changed do nothing, e.g. when rl removes consecutive dups
+ hlen = self.readline.get_current_history_length()
+ if hlen == hlen_before_cell:
+ return hlen_before_cell
+
+ for i in range(hlen - hlen_before_cell):
+ self.readline.remove_history_item(hlen - i - 1)
+ stdin_encoding = sys.stdin.encoding or "utf-8"
+ self.readline.add_history(py3compat.unicode_to_str(source_raw.rstrip(),
+ stdin_encoding))
+ return self.readline.get_current_history_length()
def interact(self, display_banner=None):
"""Closely emulate the interactive Python console."""
@@ -255,13 +272,15 @@ def interact(self, display_banner=None):
self.show_banner()
more = False
- hlen_before_cell = self.readline.get_current_history_length()
# Mark activity in the builtins
__builtin__.__dict__['__IPYTHON__active'] += 1
if self.has_readline:
self.readline_startup_hook(self.pre_readline)
+ hlen_b4_cell = self.readline.get_current_history_length()
+ else:
+ hlen_b4_cell = 0
# exit_now is set by a call to %Exit or %Quit, through the
# ask_exit callback.
@@ -293,8 +312,8 @@ def interact(self, display_banner=None):
try:
self.write('\nKeyboardInterrupt\n')
source_raw = self.input_splitter.source_raw_reset()[1]
- self._replace_rlhist_multiline(source_raw, hlen_before_cell)
- hlen_before_cell = self.readline.get_current_history_length()
+ hlen_b4_cell = \
+ self._replace_rlhist_multiline(source_raw, hlen_b4_cell)
more = False
except KeyboardInterrupt:
pass
@@ -322,9 +341,9 @@ def interact(self, display_banner=None):
self.edit_syntax_error()
if not more:
source_raw = self.input_splitter.source_raw_reset()[1]
- self._replace_rlhist_multiline(source_raw, hlen_before_cell)
- hlen_before_cell = self.readline.get_current_history_length()
self.run_cell(source_raw, store_history=True)
+ hlen_b4_cell = \
+ self._replace_rlhist_multiline(source_raw, hlen_b4_cell)
# We are off again...
__builtin__.__dict__['__IPYTHON__active'] -= 1
@@ -0,0 +1,161 @@
+# -*- coding: utf-8 -*-
+"""Tests for the key interactiveshell module.
+
+Authors
+-------
+* Julian Taylor
+"""
+#-----------------------------------------------------------------------------
+# Copyright (C) 2011 The IPython Development Team
+#
+# Distributed under the terms of the BSD License. The full license is in
+# the file COPYING, distributed as part of this software.
+#-----------------------------------------------------------------------------
+
+#-----------------------------------------------------------------------------
+# Imports
+#-----------------------------------------------------------------------------
+# stdlib
+import unittest
+
+from IPython.testing.decorators import skipif
+
+class InteractiveShellTestCase(unittest.TestCase):
+ def rl_hist_entries(self, rl, n):
+ """Get last n readline history entries as a list"""
+ return [rl.get_history_item(rl.get_current_history_length() - x)
+ for x in range(n - 1, -1, -1)]
+
+ def test_runs_without_rl(self):
+ """Test that function does not throw without readline"""
+ ip = get_ipython()
+ ip.has_readline = False
+ ip.readline = None
+ ip._replace_rlhist_multiline(u'source', 0)
+
+ @skipif(not get_ipython().has_readline, 'no readline')
+ def test_runs_without_remove_history_item(self):
+ """Test that function does not throw on windows without
+ remove_history_item"""
+ ip = get_ipython()
+ if hasattr(ip.readline, 'remove_history_item'):
+ del ip.readline.remove_history_item
+ ip._replace_rlhist_multiline(u'source', 0)
+
+ @skipif(not get_ipython().has_readline, 'no readline')
+ @skipif(not hasattr(get_ipython().readline, 'remove_history_item'),
+ 'no remove_history_item')
+ def test_replace_multiline_hist_disabled(self):
+ """Test that multiline replace does nothing if disabled"""
+ ip = get_ipython()
+ ip.multiline_history = False
+
+ ghist = [u'line1', u'line2']
+ for h in ghist:
+ ip.readline.add_history(h)
+ hlen_b4_cell = ip.readline.get_current_history_length()
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'sourc€\nsource2',
+ hlen_b4_cell)
+
+ self.assertEquals(ip.readline.get_current_history_length(),
+ hlen_b4_cell)
+ hist = self.rl_hist_entries(ip.readline, 2)
+ self.assertEquals(hist, ghist)
+
+ @skipif(not get_ipython().has_readline, 'no readline')
+ @skipif(not hasattr(get_ipython().readline, 'remove_history_item'),
+ 'no remove_history_item')
+ def test_replace_multiline_hist_adds(self):
+ """Test that multiline replace function adds history"""
+ ip = get_ipython()
+
+ hlen_b4_cell = ip.readline.get_current_history_length()
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'sourc€', hlen_b4_cell)
+
+ self.assertEquals(hlen_b4_cell,
+ ip.readline.get_current_history_length())
+
+ @skipif(not get_ipython().has_readline, 'no readline')
+ @skipif(not hasattr(get_ipython().readline, 'remove_history_item'),
+ 'no remove_history_item')
+ def test_replace_multiline_hist_keeps_history(self):
+ """Test that multiline replace does not delete history"""
+ ip = get_ipython()
+ ip.multiline_history = True
+
+ ghist = [u'line1', u'line2']
+ for h in ghist:
+ ip.readline.add_history(h)
+
+ #start cell
+ hlen_b4_cell = ip.readline.get_current_history_length()
+ # nothing added to rl history, should do nothing
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'sourc€\nsource2',
+ hlen_b4_cell)
+
+ self.assertEquals(ip.readline.get_current_history_length(),
+ hlen_b4_cell)
+ hist = self.rl_hist_entries(ip.readline, 2)
+ self.assertEquals(hist, ghist)
+
+
+ @skipif(not get_ipython().has_readline, 'no readline')
+ @skipif(not hasattr(get_ipython().readline, 'remove_history_item'),
+ 'no remove_history_item')
+ def test_replace_multiline_hist_replaces_twice(self):
+ """Test that multiline entries are replaced twice"""
+ ip = get_ipython()
+ ip.multiline_history = True
+
+ ip.readline.add_history(u'line0')
+ #start cell
+ hlen_b4_cell = ip.readline.get_current_history_length()
+ ip.readline.add_history('l€ne1')
+ ip.readline.add_history('line2')
+ #replace cell with single line
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'l€ne1\nline2',
+ hlen_b4_cell)
+ ip.readline.add_history('l€ne3')
+ ip.readline.add_history('line4')
+ #replace cell with single line
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'l€ne3\nline4',
+ hlen_b4_cell)
+
+ self.assertEquals(ip.readline.get_current_history_length(),
+ hlen_b4_cell)
+ hist = self.rl_hist_entries(ip.readline, 3)
+ self.assertEquals(hist, ['line0', 'l€ne1\nline2', 'l€ne3\nline4'])
+
+
+ @skipif(not get_ipython().has_readline, 'no readline')
+ @skipif(not hasattr(get_ipython().readline, 'remove_history_item'),
+ 'no remove_history_item')
+ def test_replace_multiline_hist_replaces_empty_line(self):
+ """Test that multiline history skips empty line cells"""
+ ip = get_ipython()
+ ip.multiline_history = True
+
+ ip.readline.add_history(u'line0')
+ #start cell
+ hlen_b4_cell = ip.readline.get_current_history_length()
+ ip.readline.add_history('l€ne1')
+ ip.readline.add_history('line2')
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'l€ne1\nline2',
+ hlen_b4_cell)
+ ip.readline.add_history('')
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'', hlen_b4_cell)
+ ip.readline.add_history('l€ne3')
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'l€ne3', hlen_b4_cell)
+ ip.readline.add_history(' ')
+ hlen_b4_cell = ip._replace_rlhist_multiline(' ', hlen_b4_cell)
+ ip.readline.add_history('\t')
+ ip.readline.add_history('\t ')
+ hlen_b4_cell = ip._replace_rlhist_multiline('\t', hlen_b4_cell)
+ ip.readline.add_history('line4')
+ hlen_b4_cell = ip._replace_rlhist_multiline(u'line4', hlen_b4_cell)
+
+ self.assertEquals(ip.readline.get_current_history_length(),
+ hlen_b4_cell)
+ hist = self.rl_hist_entries(ip.readline, 4)
+ # expect no empty cells in history
+ self.assertEquals(hist, ['line0', 'l€ne1\nline2', 'l€ne3', 'line4'])

0 comments on commit 7c0ffa5

Please sign in to comment.