Skip to content
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.

Sign up for GitHub

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

Fixed bug where async_alert() overwrites readline's incremental and non-incremental search prompts. #1330

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Fixed bug where async_alert() overwrites readline's incremental and non-incremental search prompts. #1330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
* Bug Fixes
* Fixed issue where persistent history file was not saved upon SIGHUP and SIGTERM signals.
* Multiline commands are no longer fragmented in up-arrow history.
* Fixed bug where `async_alert()` overwrites readline's incremental and non-incremental search prompts.
* This fix introduces behavior where an updated prompt won't display after an aborted search
until a user presses Enter. See [async_printing.py](https://github.com/python-cmd2/cmd2/blob/master/examples/async_printing.py)
example for how to handle this case using `Cmd.need_prompt_refresh()` and `Cmd.async_refresh_prompt()`.
* Enhancements
* Removed dependency on `attrs` and replaced with [dataclasses](https://docs.python.org/3/library/dataclasses.html)
* add `allow_clipboard` initialization parameter and attribute to disable ability to
Expand Down
8 changes: 4 additions & 4 deletions cmd2/ansi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ def async_alert_str(*, terminal_columns: int, prompt: str, line: str, cursor_off
"""Calculate the desired string, including ANSI escape codes, for displaying an asynchronous alert message.

:param terminal_columns: terminal width (number of columns)
:param prompt: prompt that is displayed on the current line
:param prompt: current onscreen prompt
:param line: current contents of the Readline line buffer
:param cursor_offset: the offset of the current cursor position within line
:param alert_msg: the message to display to the user
Expand All @@ -1071,9 +1071,9 @@ def async_alert_str(*, terminal_columns: int, prompt: str, line: str, cursor_off
# Calculate how many terminal lines are taken up by all prompt lines except for the last one.
# That will be included in the input lines calculations since that is where the cursor is.
num_prompt_terminal_lines = 0
for line in prompt_lines[:-1]:
line_width = style_aware_wcswidth(line)
num_prompt_terminal_lines += int(line_width / terminal_columns) + 1
for prompt_line in prompt_lines[:-1]:
prompt_line_width = style_aware_wcswidth(prompt_line)
num_prompt_terminal_lines += int(prompt_line_width / terminal_columns) + 1

# Now calculate how many terminal lines are take up by the input
last_prompt_line = prompt_lines[-1]
Expand Down
90 changes: 57 additions & 33 deletions cmd2/cmd2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,10 @@
from .rl_utils import (
RlType,
rl_escape_prompt,
rl_get_display_prompt,
rl_get_point,
rl_get_prompt,
rl_in_search_mode,
rl_set_prompt,
rl_type,
rl_warning,
Expand Down Expand Up @@ -353,6 +355,7 @@ def __init__(
self.hidden_commands = ['eof', '_relative_run_script']

# Initialize history
self.persistent_history_file = ''
self._persistent_history_length = persistent_history_length
self._initialize_history(persistent_history_file)

Expand Down Expand Up @@ -3295,6 +3298,12 @@ def _set_up_cmd2_readline(self) -> _SavedReadlineSettings:
"""
readline_settings = _SavedReadlineSettings()

if rl_type == RlType.GNU:
# To calculate line count when printing async_alerts, we rely on commands wider than
# the terminal to wrap across multiple lines. The default for horizontal-scroll-mode
# is "off" but a user may have overridden it in their readline initialization file.
readline.parse_and_bind("set horizontal-scroll-mode off")

if self._completion_supported():
# Set up readline for our tab completion needs
if rl_type == RlType.GNU:
Expand Down Expand Up @@ -5273,16 +5282,16 @@ class TestMyAppCase(Cmd2TestCase):
def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None: # pragma: no cover
"""
Display an important message to the user while they are at a command line prompt.
To the user it appears as if an alert message is printed above the prompt and their current input
text and cursor location is left alone.
To the user it appears as if an alert message is printed above the prompt and their
current input text and cursor location is left alone.

IMPORTANT: This function will not print an alert unless it can acquire self.terminal_lock to ensure
a prompt is onscreen. Therefore, it is best to acquire the lock before calling this function
to guarantee the alert prints and to avoid raising a RuntimeError.
This function needs to acquire self.terminal_lock to ensure a prompt is on screen.
Therefore, it is best to acquire the lock before calling this function to avoid
raising a RuntimeError.

This function is only needed when you need to print an alert while the main thread is blocking
at the prompt. Therefore, this should never be called from the main thread. Doing so will
raise a RuntimeError.
This function is only needed when you need to print an alert or update the prompt while the
main thread is blocking at the prompt. Therefore, this should never be called from the main
thread. Doing so will raise a RuntimeError.

:param alert_msg: the message to display to the user
:param new_prompt: If you also want to change the prompt that is displayed, then include it here.
Expand All @@ -5309,20 +5318,18 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:
if new_prompt is not None:
self.prompt = new_prompt

# Check if the prompt to display has changed from what's currently displayed
cur_onscreen_prompt = rl_get_prompt()
new_onscreen_prompt = self.continuation_prompt if self._at_continuation_prompt else self.prompt

if new_onscreen_prompt != cur_onscreen_prompt:
# Check if the onscreen prompt needs to be refreshed to match self.prompt.
if self.need_prompt_refresh():
update_terminal = True
rl_set_prompt(self.prompt)

if update_terminal:
import shutil

# Generate the string which will replace the current prompt and input lines with the alert
# Print a string which replaces the onscreen prompt and input lines with the alert.
terminal_str = ansi.async_alert_str(
terminal_columns=shutil.get_terminal_size().columns,
prompt=cur_onscreen_prompt,
prompt=rl_get_display_prompt(),
line=readline.get_line_buffer(),
cursor_offset=rl_get_point(),
alert_msg=alert_msg,
Expand All @@ -5333,9 +5340,6 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:
elif rl_type == RlType.PYREADLINE:
readline.rl.mode.console.write(terminal_str)

# Update Readline's prompt before we redraw it
rl_set_prompt(new_onscreen_prompt)

# Redraw the prompt and input lines below the alert
rl_force_redisplay()

Expand All @@ -5346,30 +5350,50 @@ def async_alert(self, alert_msg: str, new_prompt: Optional[str] = None) -> None:

def async_update_prompt(self, new_prompt: str) -> None: # pragma: no cover
"""
Update the command line prompt while the user is still typing at it. This is good for alerting the user to
system changes dynamically in between commands. For instance you could alter the color of the prompt to
indicate a system status or increase a counter to report an event. If you do alter the actual text of the
prompt, it is best to keep the prompt the same width as what's on screen. Otherwise the user's input text will
be shifted and the update will not be seamless.
Update the command line prompt while the user is still typing at it.

IMPORTANT: This function will not update the prompt unless it can acquire self.terminal_lock to ensure
a prompt is onscreen. Therefore, it is best to acquire the lock before calling this function
to guarantee the prompt changes and to avoid raising a RuntimeError.
This is good for alerting the user to system changes dynamically in between commands.
For instance you could alter the color of the prompt to indicate a system status or increase a
counter to report an event. If you do alter the actual text of the prompt, it is best to keep
the prompt the same width as what's on screen. Otherwise the user's input text will be shifted
and the update will not be seamless.

This function is only needed when you need to update the prompt while the main thread is blocking
at the prompt. Therefore, this should never be called from the main thread. Doing so will
raise a RuntimeError.

If user is at a continuation prompt while entering a multiline command, the onscreen prompt will
not change. However, self.prompt will still be updated and display immediately after the multiline
line command completes.
If user is at a continuation prompt while entering a multiline command, the onscreen prompt will
not change. However, self.prompt will still be updated and display immediately after the multiline
line command completes.

:param new_prompt: what to change the prompt to
:raises RuntimeError: if called from the main thread.
:raises RuntimeError: if called while another thread holds `terminal_lock`
"""
self.async_alert('', new_prompt)

def async_refresh_prompt(self) -> None: # pragma: no cover
"""
Refresh the oncreen prompt to match self.prompt.

One case where the onscreen prompt and self.prompt can get out of sync is
when async_alert() is called while a user is in search mode (e.g. Ctrl-r).
To prevent overwriting readline's onscreen search prompt, self.prompt is updated
but readline's saved prompt isn't.

Therefore when a user aborts a search, the old prompt is still on screen until they
press Enter or this method is called. Call need_prompt_refresh() in an async print
thread to know when a refresh is needed.

:raises RuntimeError: if called from the main thread.
:raises RuntimeError: if called while another thread holds `terminal_lock`
"""
self.async_alert('')

def need_prompt_refresh(self) -> bool: # pragma: no cover
"""Check whether the onscreen prompt needs to be asynchronously refreshed to match self.prompt."""
if not (vt100_support and self.use_rawinput):
return False

# Don't overwrite a readline search prompt or a continuation prompt.
return not rl_in_search_mode() and not self._at_continuation_prompt and self.prompt != rl_get_prompt()

@staticmethod
def set_window_title(title: str) -> None: # pragma: no cover
"""
Expand Down
58 changes: 50 additions & 8 deletions cmd2/rl_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ def enable_win_vt100(handle: HANDLE) -> bool:
############################################################################################################
# pyreadline3 is incomplete in terms of the Python readline API. Add the missing functions we need.
############################################################################################################
# readline.redisplay()
try:
getattr(readline, 'redisplay')
except AttributeError:
readline.redisplay = readline.rl.mode._update_line

# readline.remove_history_item()
try:
getattr(readline, 'remove_history_item')
Expand Down Expand Up @@ -200,7 +194,7 @@ def rl_get_point() -> int: # pragma: no cover


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


def rl_get_display_prompt() -> str: # pragma: no cover
"""
Get Readline's currently displayed prompt.

In GNU Readline, the displayed prompt sometimes differs from the prompt.
This occurs in functions that use the prompt string as a message area, such as incremental search.
"""
if rl_type == RlType.GNU:
encoded_prompt = ctypes.c_char_p.in_dll(readline_lib, "rl_display_prompt").value
if encoded_prompt is None:
prompt = ''
else:
prompt = encoded_prompt.decode(encoding='utf-8')
return rl_unescape_prompt(prompt)
else:
return rl_get_prompt()


def rl_set_prompt(prompt: str) -> None: # pragma: no cover
"""
Sets Readline's prompt
Expand All @@ -237,7 +249,8 @@ def rl_set_prompt(prompt: str) -> None: # pragma: no cover


def rl_escape_prompt(prompt: str) -> str:
"""Overcome bug in GNU Readline in relation to calculation of prompt length in presence of ANSI escape codes
"""
Overcome bug in GNU Readline in relation to calculation of prompt length in presence of ANSI escape codes

:param prompt: original prompt
:return: prompt safe to pass to GNU Readline
Expand Down Expand Up @@ -276,3 +289,32 @@ def rl_unescape_prompt(prompt: str) -> str:
prompt = prompt.replace(escape_start, "").replace(escape_end, "")

return prompt


def rl_in_search_mode() -> bool: # pragma: no cover
"""Check if readline is doing either an incremental (e.g. Ctrl-r) or non-incremental (e.g. Esc-p) search"""
if rl_type == RlType.GNU:
# GNU Readline defines constants that we can use to determine if in search mode.
# RL_STATE_ISEARCH 0x0000080
# RL_STATE_NSEARCH 0x0000100
IN_SEARCH_MODE = 0x0000180

readline_state = ctypes.c_int.in_dll(readline_lib, "rl_readline_state").value
return bool(IN_SEARCH_MODE & readline_state)
elif rl_type == RlType.PYREADLINE:
from pyreadline3.modes.emacs import ( # type: ignore[import]
EmacsMode,
)

# These search modes only apply to Emacs mode, which is the default.
if not isinstance(readline.rl.mode, EmacsMode):
return False

# While in search mode, the current keyevent function is set one of the following.
search_funcs = (
readline.rl.mode._process_incremental_search_keyevent,
readline.rl.mode._process_non_incremental_search_keyevent,
)
return readline.rl.mode.process_keyevent_queue[-1] in search_funcs
else:
return False
10 changes: 7 additions & 3 deletions docs/features/prompt.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ PythonScripting_ for an example of dynamically updating the prompt.
Asynchronous Feedback
---------------------

``cmd2`` provides two functions to provide asynchronous feedback to the user
``cmd2`` provides these functions to provide asynchronous feedback to the user
without interfering with the command line. This means the feedback is provided
to the user when they are still entering text at the prompt. To use this
functionality, the application must be running in a terminal that supports
Expand All @@ -52,6 +52,12 @@ all support these.
.. automethod:: cmd2.Cmd.async_update_prompt
:noindex:

.. automethod:: cmd2.Cmd.async_refresh_prompt
:noindex:

.. automethod:: cmd2.Cmd.need_prompt_refresh
:noindex:

``cmd2`` also provides a function to change the title of the terminal window.
This feature requires the application be running in a terminal that supports
VT100 control characters. Linux, Mac, and Windows 10 and greater all support
Expand All @@ -64,5 +70,3 @@ The easiest way to understand these functions is to see the AsyncPrinting_
example for a demonstration.

.. _AsyncPrinting: https://github.com/python-cmd2/cmd2/blob/master/examples/async_printing.py


11 changes: 7 additions & 4 deletions examples/async_printing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ def _alerter_thread_func(self) -> None:
self._next_alert_time = 0

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

# No alerts needed to be printed, check if the prompt changed
elif new_prompt != self.prompt:
# Otherwise check if the prompt needs to be updated or refreshed
elif self.prompt != new_prompt:
self.async_update_prompt(new_prompt)

elif self.need_prompt_refresh():
self.async_refresh_prompt()

# Don't forget to release the lock
self.terminal_lock.release()

Expand Down
Loading