Skip to content

Commit 04b0344

Browse files
committed
Fixed bug where async_alert() overwrites readline's incremental and non-incremental search prompts.
1 parent 16c6d30 commit 04b0344

File tree

5 files changed

+97
-21
lines changed

5 files changed

+97
-21
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
* Bug Fixes
55
* Fixed issue where persistent history file was not saved upon SIGHUP and SIGTERM signals.
66
* Multiline commands are no longer fragmented in up-arrow history.
7+
* Fixed bug where `async_alert()` overwrites readline's incremental and non-incremental search prompts.
8+
* This fix introduces behavior where an updated prompt won't display after an aborted search
9+
until a user presses Enter. See [async_printing.py](https://github.com/python-cmd2/cmd2/blob/master/examples/async_printing.py)
10+
example for how to handle this case using `Cmd.need_prompt_refresh()`.
711
* Enhancements
812
* Removed dependency on `attrs` and replaced with [dataclasses](https://docs.python.org/3/library/dataclasses.html)
913
* add `allow_clipboard` initialization parameter and attribute to disable ability to

cmd2/ansi.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,7 +1058,7 @@ def async_alert_str(*, terminal_columns: int, prompt: str, line: str, cursor_off
10581058
"""Calculate the desired string, including ANSI escape codes, for displaying an asynchronous alert message.
10591059
10601060
:param terminal_columns: terminal width (number of columns)
1061-
:param prompt: prompt that is displayed on the current line
1061+
:param prompt: current onscreen prompt
10621062
:param line: current contents of the Readline line buffer
10631063
:param cursor_offset: the offset of the current cursor position within line
10641064
:param alert_msg: the message to display to the user
@@ -1071,9 +1071,9 @@ def async_alert_str(*, terminal_columns: int, prompt: str, line: str, cursor_off
10711071
# Calculate how many terminal lines are taken up by all prompt lines except for the last one.
10721072
# That will be included in the input lines calculations since that is where the cursor is.
10731073
num_prompt_terminal_lines = 0
1074-
for line in prompt_lines[:-1]:
1075-
line_width = style_aware_wcswidth(line)
1076-
num_prompt_terminal_lines += int(line_width / terminal_columns) + 1
1074+
for prompt_line in prompt_lines[:-1]:
1075+
prompt_line_width = style_aware_wcswidth(prompt_line)
1076+
num_prompt_terminal_lines += int(prompt_line_width / terminal_columns) + 1
10771077

10781078
# Now calculate how many terminal lines are take up by the input
10791079
last_prompt_line = prompt_lines[-1]

cmd2/cmd2.py

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,10 @@
130130
from .rl_utils import (
131131
RlType,
132132
rl_escape_prompt,
133+
rl_get_display_prompt,
133134
rl_get_point,
134135
rl_get_prompt,
136+
rl_in_search_mode,
135137
rl_set_prompt,
136138
rl_type,
137139
rl_warning,
@@ -3295,6 +3297,12 @@ def _set_up_cmd2_readline(self) -> _SavedReadlineSettings:
32953297
"""
32963298
readline_settings = _SavedReadlineSettings()
32973299

3300+
if rl_type == RlType.GNU:
3301+
# To calculate line count when printing async_alerts, we rely on commands wider than
3302+
# the terminal to wrap across multiple lines. The default for horizontal-scroll-mode
3303+
# is "off" but a user may have overridden it in their readline initialization file.
3304+
readline.parse_and_bind("set horizontal-scroll-mode off")
3305+
32983306
if self._completion_supported():
32993307
# Set up readline for our tab completion needs
33003308
if rl_type == RlType.GNU:
@@ -5277,7 +5285,7 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:
52775285
text and cursor location is left alone.
52785286
52795287
IMPORTANT: This function will not print an alert unless it can acquire self.terminal_lock to ensure
5280-
a prompt is onscreen. Therefore, it is best to acquire the lock before calling this function
5288+
a prompt is on screen. Therefore, it is best to acquire the lock before calling this function
52815289
to guarantee the alert prints and to avoid raising a RuntimeError.
52825290
52835291
This function is only needed when you need to print an alert while the main thread is blocking
@@ -5309,20 +5317,18 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:
53095317
if new_prompt is not None:
53105318
self.prompt = new_prompt
53115319

5312-
# Check if the prompt to display has changed from what's currently displayed
5313-
cur_onscreen_prompt = rl_get_prompt()
5314-
new_onscreen_prompt = self.continuation_prompt if self._at_continuation_prompt else self.prompt
5315-
5316-
if new_onscreen_prompt != cur_onscreen_prompt:
5320+
# Check if the onscreen prompt needs to be refreshed to match self.prompt.
5321+
if self.need_prompt_refresh():
53175322
update_terminal = True
5323+
rl_set_prompt(self.prompt)
53185324

53195325
if update_terminal:
53205326
import shutil
53215327

5322-
# Generate the string which will replace the current prompt and input lines with the alert
5328+
# Print a string which replaces the onscreen prompt and input lines with the alert.
53235329
terminal_str = ansi.async_alert_str(
53245330
terminal_columns=shutil.get_terminal_size().columns,
5325-
prompt=cur_onscreen_prompt,
5331+
prompt=rl_get_display_prompt(),
53265332
line=readline.get_line_buffer(),
53275333
cursor_offset=rl_get_point(),
53285334
alert_msg=alert_msg,
@@ -5333,9 +5339,6 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:
53335339
elif rl_type == RlType.PYREADLINE:
53345340
readline.rl.mode.console.write(terminal_str)
53355341

5336-
# Update Readline's prompt before we redraw it
5337-
rl_set_prompt(new_onscreen_prompt)
5338-
53395342
# Redraw the prompt and input lines below the alert
53405343
rl_force_redisplay()
53415344

@@ -5370,6 +5373,26 @@ def async_update_prompt(self, new_prompt: str) -> None: # pragma: no cover
53705373
"""
53715374
self.async_alert('', new_prompt)
53725375

5376+
def need_prompt_refresh(self) -> bool: # pragma: no cover
5377+
"""
5378+
Used by async print threads to check whether the onscreen prompt needs to be
5379+
refreshed to match self.prompt.
5380+
5381+
One case where the onscreen prompt and self.prompt can get out of sync is
5382+
when async_alert() is called while a user is in search mode (e.g. Ctrl-r).
5383+
To prevent overwriting readline's onscreen search prompt, self.prompt is updated
5384+
but readline's saved prompt isn't.
5385+
5386+
Therefore when a user aborts a search, the old prompt is still on screen until they
5387+
press Enter or async_alert() is called again. Use this function in an async print
5388+
thread to know when to make that extra call to async_alert().
5389+
"""
5390+
if not (vt100_support and self.use_rawinput):
5391+
return False
5392+
5393+
# Don't overwrite a readline search prompt or a continuation prompt.
5394+
return not rl_in_search_mode() and not self._at_continuation_prompt and self.prompt != rl_get_prompt()
5395+
53735396
@staticmethod
53745397
def set_window_title(title: str) -> None: # pragma: no cover
53755398
"""

cmd2/rl_utils.py

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ def rl_get_point() -> int: # pragma: no cover
200200

201201

202202
def rl_get_prompt() -> str: # pragma: no cover
203-
"""Gets Readline's current prompt"""
203+
"""Get Readline's prompt"""
204204
if rl_type == RlType.GNU:
205205
encoded_prompt = ctypes.c_char_p.in_dll(readline_lib, "rl_prompt").value
206206
if encoded_prompt is None:
@@ -221,6 +221,24 @@ def rl_get_prompt() -> str: # pragma: no cover
221221
return rl_unescape_prompt(prompt)
222222

223223

224+
def rl_get_display_prompt() -> str: # pragma: no cover
225+
"""
226+
Get Readline's currently displayed prompt.
227+
228+
In GNU Readline, the displayed prompt sometimes differs from the prompt.
229+
This occurs in functions that use the prompt string as a message area, such as incremental search.
230+
"""
231+
if rl_type == RlType.GNU:
232+
encoded_prompt = ctypes.c_char_p.in_dll(readline_lib, "rl_display_prompt").value
233+
if encoded_prompt is None:
234+
prompt = ''
235+
else:
236+
prompt = encoded_prompt.decode(encoding='utf-8')
237+
return rl_unescape_prompt(prompt)
238+
else:
239+
return rl_get_prompt()
240+
241+
224242
def rl_set_prompt(prompt: str) -> None: # pragma: no cover
225243
"""
226244
Sets Readline's prompt
@@ -237,7 +255,8 @@ def rl_set_prompt(prompt: str) -> None: # pragma: no cover
237255

238256

239257
def rl_escape_prompt(prompt: str) -> str:
240-
"""Overcome bug in GNU Readline in relation to calculation of prompt length in presence of ANSI escape codes
258+
"""
259+
Overcome bug in GNU Readline in relation to calculation of prompt length in presence of ANSI escape codes
241260
242261
:param prompt: original prompt
243262
:return: prompt safe to pass to GNU Readline
@@ -276,3 +295,32 @@ def rl_unescape_prompt(prompt: str) -> str:
276295
prompt = prompt.replace(escape_start, "").replace(escape_end, "")
277296

278297
return prompt
298+
299+
300+
def rl_in_search_mode() -> bool:
301+
"""Check if readline is doing either an incremental (e.g. Ctrl-r) or non-incremental (e.g. Esc-p) search"""
302+
if rl_type == RlType.GNU:
303+
# GNU Readline defines constants that we can use to determine if in search mode.
304+
# RL_STATE_ISEARCH 0x0000080
305+
# RL_STATE_NSEARCH 0x0000100
306+
IN_SEARCH_MODE = 0x0000180
307+
308+
readline_state = ctypes.c_int.in_dll(readline_lib, "rl_readline_state").value
309+
return bool(IN_SEARCH_MODE & readline_state)
310+
elif rl_type == RlType.PYREADLINE:
311+
from pyreadline3.modes.emacs import ( # type: ignore[import]
312+
EmacsMode,
313+
)
314+
315+
# These search modes only apply to Emacs mode, which is the default.
316+
if not isinstance(readline.rl.mode, EmacsMode):
317+
return False
318+
319+
# While in search mode, the current keyevent function is set one of the following.
320+
search_funcs = (
321+
readline.rl.mode._process_incremental_search_keyevent,
322+
readline.rl.mode._process_non_incremental_search_keyevent,
323+
)
324+
return readline.rl.mode.process_keyevent_queue[-1] in search_funcs
325+
else:
326+
return False

examples/async_printing.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ def _alerter_thread_func(self) -> None:
173173
self._next_alert_time = 0
174174

175175
while not self._stop_event.is_set():
176-
# Always acquire terminal_lock before printing alerts or updating the prompt
177-
# To keep the app responsive, do not block on this call
176+
# Always acquire terminal_lock before printing alerts or updating the prompt.
177+
# To keep the app responsive, do not block on this call.
178178
if self.terminal_lock.acquire(blocking=False):
179179
# Get any alerts that need to be printed
180180
alert_str = self._generate_alert_str()
@@ -189,8 +189,9 @@ def _alerter_thread_func(self) -> None:
189189
new_title = "Alerts Printed: {}".format(self._alert_count)
190190
self.set_window_title(new_title)
191191

192-
# No alerts needed to be printed, check if the prompt changed
193-
elif new_prompt != self.prompt:
192+
# There are no alerts to print, but we should still check
193+
# if the onscreen prompt needs to be updated.
194+
elif self.prompt != new_prompt or self.need_prompt_refresh():
194195
self.async_update_prompt(new_prompt)
195196

196197
# Don't forget to release the lock

0 commit comments

Comments
 (0)