From 77ca92a5a096ae5a528093595f40575113caf2f0 Mon Sep 17 00:00:00 2001 From: Aya Elsayed Date: Tue, 21 May 2024 21:03:13 +0100 Subject: [PATCH 1/6] gh-118911: Trailing whitespace in a block shouldnt prevent the user from terminating the code block --- Lib/_pyrepl/historical_reader.py | 2 +- Lib/_pyrepl/readline.py | 25 ++++++++++++---- Lib/test/test_pyrepl/test_pyrepl.py | 9 +++--- Lib/test/test_pyrepl/test_reader.py | 45 ++++++++++++++++++++++++++++- 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/Lib/_pyrepl/historical_reader.py b/Lib/_pyrepl/historical_reader.py index eef7d901b083ef..121de33da5052f 100644 --- a/Lib/_pyrepl/historical_reader.py +++ b/Lib/_pyrepl/historical_reader.py @@ -259,7 +259,7 @@ def select_item(self, i: int) -> None: self.transient_history[self.historyi] = self.get_unicode() buf = self.transient_history.get(i) if buf is None: - buf = self.history[i] + buf = self.history[i].rstrip() self.buffer = list(buf) self.historyi = i self.pos = len(self.buffer) diff --git a/Lib/_pyrepl/readline.py b/Lib/_pyrepl/readline.py index 0adecf235a4eb4..a7fb0c1a56a21f 100644 --- a/Lib/_pyrepl/readline.py +++ b/Lib/_pyrepl/readline.py @@ -219,16 +219,29 @@ def do(self) -> None: # if there are already several lines and the cursor # is not on the last one, always insert a new \n. text = r.get_unicode() + if "\n" in r.buffer[r.pos :] or ( r.more_lines is not None and r.more_lines(text) ): + def _whitespace_before_pos(): + before_idx = r.pos - 1 + while before_idx > 0 and text[before_idx].isspace(): + before_idx -= 1 + return text[before_idx : r.pos].count("\n") > 0 # - # auto-indent the next line like the previous line - prevlinestart, indent = _get_previous_line_indent(r.buffer, r.pos) - r.insert("\n") - if not self.reader.paste_mode and indent: - for i in range(prevlinestart, prevlinestart + indent): - r.insert(r.buffer[i]) + # if there's already a new line before the cursor then + # even if the cursor is followed by whitespace, we assume + # the user is trying to terminate the block + if _whitespace_before_pos() and text[r.pos:].isspace(): + self.finish = True + else: + # + # auto-indent the next line like the previous line + prevlinestart, indent = _get_previous_line_indent(r.buffer, r.pos) + r.insert("\n") + if not self.reader.paste_mode and indent: + for i in range(prevlinestart, prevlinestart + indent): + r.insert(r.buffer[i]) elif not self.reader.paste_mode: self.finish = True else: diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index bc0a9975e34e00..da855a826603da 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -317,14 +317,13 @@ def test_multiline_edit(self): [ Event(evt="key", data="up", raw=bytearray(b"\x1bOA")), Event(evt="key", data="up", raw=bytearray(b"\x1bOA")), - Event(evt="key", data="up", raw=bytearray(b"\x1bOA")), - Event(evt="key", data="right", raw=bytearray(b"\x1bOC")), - Event(evt="key", data="right", raw=bytearray(b"\x1bOC")), - Event(evt="key", data="right", raw=bytearray(b"\x1bOC")), + Event(evt="key", data="left", raw=bytearray(b"\x1bOD")), + Event(evt="key", data="left", raw=bytearray(b"\x1bOD")), + Event(evt="key", data="left", raw=bytearray(b"\x1bOD")), Event(evt="key", data="backspace", raw=bytearray(b"\x7f")), Event(evt="key", data="g", raw=bytearray(b"g")), Event(evt="key", data="down", raw=bytearray(b"\x1bOB")), - Event(evt="key", data="down", raw=bytearray(b"\x1bOB")), + Event(evt="key", data="\n", raw=bytearray(b"\n")), Event(evt="key", data="\n", raw=bytearray(b"\n")), ], ) diff --git a/Lib/test/test_pyrepl/test_reader.py b/Lib/test/test_pyrepl/test_reader.py index dc7d8a5ba97cda..eb6ef75662af63 100644 --- a/Lib/test/test_pyrepl/test_reader.py +++ b/Lib/test/test_pyrepl/test_reader.py @@ -1,7 +1,8 @@ import itertools +import functools from unittest import TestCase -from .support import handle_all_events, handle_events_narrow_console, code_to_events +from .support import handle_all_events, handle_events_narrow_console, code_to_events, prepare_reader from _pyrepl.console import Event @@ -133,3 +134,45 @@ def test_up_arrow_after_ctrl_r(self): reader, _ = handle_all_events(events) self.assert_screen_equals(reader, "") + + def test_newline_within_block_trailing_whitespace(self): + # fmt: off + code = ( + "def foo():\n" + " a = 1\n" + ) + # fmt: on + + events = itertools.chain( + code_to_events(code), + [ + # go to the end of the first line + Event(evt="key", data="up", raw=bytearray(b"\x1bOA")), + Event(evt="key", data="up", raw=bytearray(b"\x1bOA")), + Event(evt="key", data="\x05", raw=bytearray(b"\x1bO5")), + # new lines in-block shouldn't terminate the block + Event(evt="key", data="\n", raw=bytearray(b"\n")), + Event(evt="key", data="\n", raw=bytearray(b"\n")), + # end of line 2 + Event(evt="key", data="down", raw=bytearray(b"\x1bOB")), + Event(evt="key", data="\x05", raw=bytearray(b"\x1bO5")), + # a double new line in-block should terminate the block + # even if its followed by whitespace + Event(evt="key", data="\n", raw=bytearray(b"\n")), + Event(evt="key", data="\n", raw=bytearray(b"\n")), + ], + ) + + no_paste_reader = functools.partial(prepare_reader, paste_mode=False) + reader, _ = handle_all_events(events, prepare_reader=no_paste_reader) + + expected = ( + "def foo():\n" + "\n" + "\n" + " a = 1\n" + " \n" + " " # HistoricalReader will trim trailing whitespace + ) + self.assert_screen_equals(reader, expected) + self.assertEqual(reader.finished, True) From edbf327a4069efa61888d919a627dd8f5140c65b Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 21 May 2024 20:13:24 +0000 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst diff --git a/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst b/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst new file mode 100644 index 00000000000000..c329bc0a549472 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst @@ -0,0 +1,2 @@ +- If the user hits enter twice, they are able to exit out of the block, even if there's trailing whitespace. +- Trim trailing whitespace in the HistoricalReader to match IPython's behavior From 4996c5ea3be47dd062548fa07f5c46639cd1ffab Mon Sep 17 00:00:00 2001 From: Aya Elsayed Date: Tue, 21 May 2024 21:28:42 +0100 Subject: [PATCH 3/6] for clarity --- Lib/_pyrepl/readline.py | 4 ++-- Lib/test/test_pyrepl/test_reader.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/_pyrepl/readline.py b/Lib/_pyrepl/readline.py index a7fb0c1a56a21f..a1aafda45a504d 100644 --- a/Lib/_pyrepl/readline.py +++ b/Lib/_pyrepl/readline.py @@ -223,7 +223,7 @@ def do(self) -> None: if "\n" in r.buffer[r.pos :] or ( r.more_lines is not None and r.more_lines(text) ): - def _whitespace_before_pos(): + def _newline_before_pos(): before_idx = r.pos - 1 while before_idx > 0 and text[before_idx].isspace(): before_idx -= 1 @@ -232,7 +232,7 @@ def _whitespace_before_pos(): # if there's already a new line before the cursor then # even if the cursor is followed by whitespace, we assume # the user is trying to terminate the block - if _whitespace_before_pos() and text[r.pos:].isspace(): + if _newline_before_pos() and text[r.pos:].isspace(): self.finish = True else: # diff --git a/Lib/test/test_pyrepl/test_reader.py b/Lib/test/test_pyrepl/test_reader.py index eb6ef75662af63..c5d892a9de5504 100644 --- a/Lib/test/test_pyrepl/test_reader.py +++ b/Lib/test/test_pyrepl/test_reader.py @@ -175,4 +175,4 @@ def test_newline_within_block_trailing_whitespace(self): " " # HistoricalReader will trim trailing whitespace ) self.assert_screen_equals(reader, expected) - self.assertEqual(reader.finished, True) + self.assertTrue(reader.finished) From 6c0682b142a98705eab6aa1599247f9b5bd14b29 Mon Sep 17 00:00:00 2001 From: Aya Elsayed Date: Tue, 21 May 2024 21:32:07 +0100 Subject: [PATCH 4/6] lint --- Lib/test/test_pyrepl/test_reader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_pyrepl/test_reader.py b/Lib/test/test_pyrepl/test_reader.py index c5d892a9de5504..bf8f7de3b129bc 100644 --- a/Lib/test/test_pyrepl/test_reader.py +++ b/Lib/test/test_pyrepl/test_reader.py @@ -146,7 +146,7 @@ def test_newline_within_block_trailing_whitespace(self): events = itertools.chain( code_to_events(code), [ - # go to the end of the first line + # go to the end of the first line Event(evt="key", data="up", raw=bytearray(b"\x1bOA")), Event(evt="key", data="up", raw=bytearray(b"\x1bOA")), Event(evt="key", data="\x05", raw=bytearray(b"\x1bO5")), From 4e62b964a744019f2d73963fc488766920813bb4 Mon Sep 17 00:00:00 2001 From: Aya Elsayed Date: Tue, 21 May 2024 22:11:54 +0100 Subject: [PATCH 5/6] reword NEWS.d --- .../Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst b/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst index c329bc0a549472..95a66c12fa74e2 100644 --- a/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst +++ b/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst @@ -1,2 +1,2 @@ -- If the user hits enter twice, they are able to exit out of the block, even if there's trailing whitespace. -- Trim trailing whitespace in the HistoricalReader to match IPython's behavior +To match IPython's behaviour, updated `maybe-accept`'s logic so that if the user hits enter twice, they are able to terminate the block even if there's trailing whitespace. +Also, trimming trailing whitespace in HistoricalReader so that when the user hits arrow up, the cursor is on the last functional line. This matches IPython's behavior. From 27ad76cb9d6827686b331126f1e9eb9713916e9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 22 May 2024 07:29:09 +0200 Subject: [PATCH 6/6] Fix ReST syntax in blurb, reword a bit --- .../Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst b/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst index 95a66c12fa74e2..4f15c1b67c9774 100644 --- a/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst +++ b/Misc/NEWS.d/next/Library/2024-05-21-20-13-23.gh-issue-118911.iG8nMq.rst @@ -1,2 +1,5 @@ -To match IPython's behaviour, updated `maybe-accept`'s logic so that if the user hits enter twice, they are able to terminate the block even if there's trailing whitespace. -Also, trimming trailing whitespace in HistoricalReader so that when the user hits arrow up, the cursor is on the last functional line. This matches IPython's behavior. +In PyREPL, updated ``maybe-accept``'s logic so that if the user hits +:kbd:`Enter` twice, they are able to terminate the block even if there's +trailing whitespace. Also, now when the user hits arrow up, the cursor +is on the last functional line. This matches IPython's behavior. +Patch by Aya Elsayed.