Skip to content

Ability to suppress control-d exit behavior#1549

Merged
rolandwalker merged 1 commit intomainfrom
RW/control-d-exit-behavior
Feb 16, 2026
Merged

Ability to suppress control-d exit behavior#1549
rolandwalker merged 1 commit intomainfrom
RW/control-d-exit-behavior

Conversation

@rolandwalker
Copy link
Contributor

Description

Arguably the default should be changed since it can be confusing to new users. But exiting on control-d/EOF is also the default for eg bash.

Ringing the bell when no action can be taken is consistent with other control keys on the empty line.

Tests would be nice! But would require an alternate myclirc and simulated keypresses, which I don't know how to do.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

@rolandwalker rolandwalker self-assigned this Feb 14, 2026
Copy link
Contributor

@scottnemes scottnemes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified new option controls the ctrl-d behavior as expected.

Arguably the default should be changed since it can be confusing to new
users.  But exiting on control-d/EOF is also the default for _eg_ bash.

Ringing the bell when no action can be taken is consistent with other
control keys on the empty line.
@rolandwalker rolandwalker force-pushed the RW/control-d-exit-behavior branch from dfec2e9 to 8f07da7 Compare February 16, 2026 09:05
@github-actions
Copy link

Review summary

I focused on correctness/regressions, security, and missing tests. No security issues spotted. Two potential behavior/regression concerns and a test gap below.

Findings

  1. Potential keybinding regression in non‑Emacs modes
    The new c-d binding is not gated by emacs_mode, so it will also fire in vi mode whenever the buffer is empty. That could override prompt_toolkit’s vi‑mode c-d behavior (if any) and may be unexpected for users running vi keybindings.
    Action: consider adding a mode filter (e.g., emacs_mode or vi_insert_mode depending on intended behavior), or explicitly document that this applies in all modes.
    File: mycli/key_bindings.py

  2. Config typo silently disables exit
    Any value other than the literal 'exit' falls into the else branch and rings the bell, which effectively disables exit on Ctrl‑D without warning. A typo like control_d = exiit will be hard to diagnose.
    Action: treat unknown values as 'exit' (or log a warning) to avoid silent misconfiguration.
    File: mycli/key_bindings.py

Missing tests

  1. Keybinding behavior for control_d
    Add tests that simulate Ctrl‑D on an empty buffer for both modes:
    • control_d = exit should trigger event.app.exit(exception=EOFError, ...)
    • control_d = none should not exit and should bell
      This can likely be done using prompt_toolkit’s testing utilities and a temporary config override.
      Files: mycli/key_bindings.py, test coverage (new test file)

If you want, I can sketch a minimal test harness for prompt_toolkit keybindings based on the existing test setup.

@rolandwalker rolandwalker merged commit 0c6288f into main Feb 16, 2026
10 checks passed
@rolandwalker rolandwalker deleted the RW/control-d-exit-behavior branch February 16, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants