Skip to content

feat(bind): extend key binding support#740

Merged
reubeno merged 2 commits into
mainfrom
more-bindings
Nov 7, 2025
Merged

feat(bind): extend key binding support#740
reubeno merged 2 commits into
mainfrom
more-bindings

Conversation

@reubeno
Copy link
Copy Markdown
Owner

@reubeno reubeno commented Nov 7, 2025

Extends the bind builtin to support binding key sequences to readline functions (previously it only supported shell commands). Bindings to more complex macros remain unimplemented.

  • Adds support for parsing and binding key sequences to readline functions (e.g., "\C-a":beginning-of-line)
  • Implements translation of InputFunction variants to reedline events in the interactive layer
  • Refactors the parser to distinguish between shell command bindings and readline target bindings

@reubeno reubeno requested a review from Copilot November 7, 2025 09:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the bind builtin to support binding key sequences to readline functions (not just shell commands). It introduces a distinction between shell command bindings and readline function/command bindings, adds parsing for both types, implements translations of readline functions to reedline events, and includes comprehensive test coverage.

  • Adds support for parsing and binding key sequences to readline functions (e.g., "\C-a":beginning-of-line)
  • Implements translation of InputFunction variants to reedline events in the interactive layer
  • Refactors the parser to distinguish between shell command bindings and readline target bindings

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
brush-parser/src/readline_binding.rs Splits binding types into KeySequenceShellCommandBinding (for shell commands) and KeySequenceReadlineBinding (for readline functions/commands); adds parsing functions for both
brush-interactive/src/reedline/edit_mode.rs Implements translate_input_function_to_reedline_event() to map InputFunction variants to reedline events; adds support for Repaint event
brush-interactive/src/interactive_shell.rs Updates buffer handling logic to support programmatically-generated edit buffers from executed commands
brush-core/src/sys/unix/input.rs Renames get_key_from_key_code() to try_get_key_from_key_code() and changes return type from Result to Option
brush-core/src/error.rs Adds new error variant UnknownKeyBindingFunction for invalid function names
brush-builtins/src/bind.rs Refactors to use new parser functions, adds parse_key_sequence_and_readline_target(), implements bind_key_sequence_to_readline_target(), adds escape sequence handling, includes unit tests

Comment thread brush-interactive/src/interactive_shell.rs Outdated
Comment thread brush-parser/src/readline_binding.rs
Comment thread brush-builtins/src/bind.rs
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 7, 2025

Public API changes for crate: brush-core

Removed items

-pub fn brush_core::sys::input::get_key_from_key_code(key_code: &[u8]) -> core::result::Result<brush_core::interfaces::Key, brush_core::error::Error>

Added items

+pub brush_core::error::ErrorKind::UnknownKeyBindingFunction(alloc::string::String)
+pub fn brush_core::sys::input::try_get_key_from_key_code(key_code: &[u8]) -> core::option::Option<brush_core::interfaces::Key>
+pub brush_core::ErrorKind::UnknownKeyBindingFunction(alloc::string::String)

Public API changes for crate: brush-parser

Removed items

-pub struct brush_parser::readline_binding::KeySequenceBinding
-pub brush_parser::readline_binding::KeySequenceBinding::command: alloc::string::String
-pub brush_parser::readline_binding::KeySequenceBinding::seq: brush_parser::readline_binding::KeySequence
-pub fn brush_parser::readline_binding::parse_key_sequence_binding(input: &str) -> core::result::Result<brush_parser::readline_binding::KeySequenceBinding, brush_parser::BindingParseError>

Added items

+pub enum brush_parser::readline_binding::ReadlineTarget
+pub brush_parser::readline_binding::ReadlineTarget::Command(alloc::string::String)
+pub brush_parser::readline_binding::ReadlineTarget::Function(alloc::string::String)
+pub struct brush_parser::readline_binding::KeySequenceReadlineBinding
+pub brush_parser::readline_binding::KeySequenceReadlineBinding::seq: brush_parser::readline_binding::KeySequence
+pub brush_parser::readline_binding::KeySequenceReadlineBinding::target: brush_parser::readline_binding::ReadlineTarget
+pub struct brush_parser::readline_binding::KeySequenceShellCommandBinding
+pub brush_parser::readline_binding::KeySequenceShellCommandBinding::seq: brush_parser::readline_binding::KeySequence
+pub brush_parser::readline_binding::KeySequenceShellCommandBinding::shell_cmd: alloc::string::String
+pub fn brush_parser::readline_binding::parse_key_sequence_readline_binding(input: &str) -> core::result::Result<brush_parser::readline_binding::KeySequenceReadlineBinding, brush_parser::BindingParseError>
+pub fn brush_parser::readline_binding::parse_key_sequence_shell_cmd_binding(input: &str) -> core::result::Result<brush_parser::readline_binding::KeySequenceShellCommandBinding, brush_parser::BindingParseError>

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 17.70 μs 17.23 μs -0.47 μs 🟢 -2.65%
eval_arithmetic 0.15 μs 0.15 μs -0.00 μs ⚪ Unchanged
expand_one_string 1.96 μs 1.98 μs 0.03 μs ⚪ Unchanged
for_loop 22.21 μs 22.60 μs 0.39 μs 🟠 +1.75%
function_call 2.57 μs 2.55 μs -0.02 μs ⚪ Unchanged
instantiate_shell 58.16 μs 58.15 μs -0.01 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 24026.51 μs 24660.09 μs 633.58 μs ⚪ Unchanged
parse_bash_completion 2068.23 μs 2053.15 μs -15.08 μs ⚪ Unchanged
parse_sample_script 2.02 μs 2.01 μs -0.01 μs ⚪ Unchanged
run_echo_builtin_command 15.87 μs 16.54 μs 0.67 μs ⚪ Unchanged
run_one_external_command 2414.83 μs 2378.24 μs -36.59 μs 🟢 -1.52%
tokenize_sample_script 3.55 μs 3.52 μs -0.03 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-builtins/src/bind.rs 🔴 0% 🔴 34.65% 🟢 34.65%
brush-core/src/options.rs 🟢 89% 🟢 88% 🔴 -1%
brush-core/src/shell.rs 🟠 73.69% 🟢 75.24% 🟢 1.55%
brush-core/src/sys/unix/input.rs 🔴 0% 🔴 24.49% 🟢 24.49%
brush-interactive/src/interactive_shell.rs 🟠 61.6% 🟠 63.49% 🟢 1.89%
brush-parser/src/readline_binding.rs 🔴 30% 🟠 74.12% 🟢 44.12%
Overall Coverage 🟢 71.52% 🟢 71.71% 🟢 0.19%

Minimum allowed coverage is 70%, this run produced 71.71%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1345 63.77
❗️ Error 213 10.10
❌ Fail 182 8.63
⏩ Skip 354 16.79
❎ Expected Fail 14 0.66
✔️ Unexpected Pass 1 0.05
📊 Total 2109 100.00

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 7, 2025

Test Results

    3 files     23 suites   7m 16s ⏱️
1 110 tests 1 110 ✅ 0 💤 0 ❌
3 310 runs  3 310 ✅ 0 💤 0 ❌

Results for commit 0c69c89.

@reubeno reubeno marked this pull request as ready for review November 7, 2025 09:46
@reubeno reubeno changed the title feat: extend binding support feat(bind): extend key binding support Nov 7, 2025
@reubeno
Copy link
Copy Markdown
Owner Author

reubeno commented Nov 7, 2025

I've done some manual testing. This change makes things better for fzf shell integration. Sadly, atuin shell integration isn't looking as good as it did previously, but that seems more to be owing to upstream changes in that project's bash integration script.

Two steps forward, one step back.

With that said, this looks to be a general improvement so I'm going to move forward with merging it. There are still errors from readline "macros", though.

@reubeno reubeno merged commit e0f3b87 into main Nov 7, 2025
42 checks passed
@reubeno reubeno deleted the more-bindings branch November 7, 2025 15: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