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.

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

Merge key mapping with key bindings #715

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented Jun 2, 2024

Depends on #713
Fourth step of #708

Combines KeyActor::XXX_MAPPING(single byte to method symbol) with key_bindings(escape sequence to method symbol)

This will simplify the implementation of keys in LineEditor.
We can remove key.instance_of?(String) key.is_a?(Symbol) check.
Reline::Key structure will be simple.

# before
EOF:          Key{ char: nil, combined_char: nil, with_meta: false}
Normal key:   Key{ char: number, combined_char: number, with_meta: false}
ESC+key:      Key{ char: number, combined_char: number|0x80, with_meta: true}
Binding key:  Key{ char: symbol, combined_char: symbol, with_meta: false}

# after
EOF:   Key{ char: nil, combined_char: nil}
Other: Key{ char: string, method_symbol: symbol}

Future refactor

Key can contain string.
This means we can assign bracketed-pasted string to a key. bracketed_paste and quoted_insert will be simple.
Simplifies bracketed-paste and completion handling, undo-redo handling. Simplifies quoted_insert waiting_proc mechanism and fixes bug.

key.char = read_bracketed_paste if key.method_symbol == :bracketed_paste_start
key.char = read_next_valid_char if key.method_symbol == :quoted_insert
line_editor.update(key)

Implementing compress-meta will be significantly easy.

class Reline::KeyActor::Base
  def add_mappings(mappings)
    ...
    128.times do |key|
      ...
      add([27, key], meta_func)
      add([key | 0x80], meta_func) if compress_meta_enabled # Just add this line
    end
  end
end

@tompng tompng force-pushed the merge_mapping_bindings branch 2 times, most recently from 55db8dc to 2c2bb1f Compare June 5, 2024 05:02
@tompng tompng force-pushed the merge_mapping_bindings branch 2 times, most recently from 02664be to 209a372 Compare June 11, 2024 16:54
@tompng tompng force-pushed the merge_mapping_bindings branch 4 times, most recently from 1a4466d to 8dd9a20 Compare November 27, 2024 04:49
@tompng tompng marked this pull request as ready for review November 27, 2024 05:34
@tompng tompng force-pushed the merge_mapping_bindings branch 2 times, most recently from 83943ef to 51e578a Compare November 28, 2024 13:23
@tompng tompng force-pushed the merge_mapping_bindings branch from 51e578a to aac77dc Compare December 6, 2024 06:47
@tompng tompng force-pushed the merge_mapping_bindings branch from aac77dc to 0cf6a8e Compare December 6, 2024 06:56
Copy link
Member

@ima1zumi ima1zumi left a comment

Choose a reason for hiding this comment

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

LGTM!!

@ima1zumi ima1zumi merged commit 6a7e249 into ruby:master Dec 6, 2024
40 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Dec 6, 2024
@tompng tompng deleted the merge_mapping_bindings branch December 7, 2024 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants