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

Refactor history move and history search #651

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented Mar 19, 2024

Fixes #596

Add method move_history

Using this will prevent inconsistency of @history_pointer and @line_backup_in_history that caused #596. It also reduce duplicated code.

Refactored generate_searcher used for incremental search.

generate_searcher is used in incremental_search_history
Both searcher and incremental_search_history was modifying LineEditor's state, making hard to understand the behavior.
Now searcher only searches, does not mutate LineEditor's state.
(Also changed the implementation from Fiber to lambda. )

@tompng tompng force-pushed the history_move_search_refactor branch from 133dd11 to 139be1f Compare March 20, 2024 05:23
@tompng tompng force-pushed the history_move_search_refactor branch from 139be1f to 498007a Compare March 26, 2024 02:29
@ima1zumi ima1zumi added the bug Something isn't working label Apr 1, 2024
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.

Great work! Thanks!

Comment on lines +1701 to +1709
private def ed_search_prev_history(key, arg: 1)
substr = current_line.byteslice(0, @byte_pointer)
return if @history_pointer == 0
return if @history_pointer.nil? && substr.empty? && !current_line.empty?

history_range = 0...(@history_pointer || Reline::HISTORY.size)
h_pointer, line_index = search_history(substr, history_range.reverse_each)
return unless h_pointer
move_history(h_pointer, line: line_index || :start, cursor: @byte_pointer)
Copy link
Member

Choose a reason for hiding this comment

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

It's so simple! 🤩

@ima1zumi ima1zumi merged commit 90e43e0 into ruby:master Apr 16, 2024
40 checks passed
@tompng tompng deleted the history_move_search_refactor branch April 16, 2024 14:15
@kachick kachick mentioned this pull request Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Reline crashes with PAGEUP and DOWN_ARROW
2 participants