In ed_search_[prev|next]_history, make the cursor come to the end of the line when there is no search substr

[QWYNG]

fix #618

Overview

To solve the problem, I have modified the cursor behavior to move to the end of the line if the search string is empty.
Please let me know if there is anything I am missing.

Thank you for maintaining this great Ruby feature.

[tompng]

This part should be ('12aaa', '')
#618 might be harder than I first thought.

I checked GNU Readline.

ruby -rreadline -rreline -e "raise if Readline==Reline; loop{p Readline.readline 'prompt> ', true}"

prompt> █
ed_search_prev_history, search with empty string
prompt> 12345█
(A) ed_search_prev_history, search with empty string
prompt> 12aaa█

prompt> █
ed_search_prev_history, search with empty string
prompt> 12345█
Move cursor left and right. Continuous search_xxx_history is reset.
prompt> 12345█
(B) ed_search_prev_history, search with "12345", search fails
prompt> 12345█

There is a difference between (A) and (B). Same input, same cursor position, but there is a hidden state. We need to introduce a new state (instance variable) to achieve this.

[tompng]

Adding new instance variable for many def ed_[key_action_name] will make maintainability poor, so I think we should introduce a general-purpose feature for passing state to next action.

One idea is:

def input_key
  ...
  # process_key() or normal_char() is called here
  ...
  @prev_action_state, @next_action_state = @next_action_state, nil
end

Key action method can read @prev_action_state.
If the action want to pass a state to next action, set @next_action_state to something like [state_type, state_value].

[QWYNG]

Thank you for your clear comment. To replicate the behavior of Readline, it is best to include information about the before and after states of the current operation. I will give it a try

[QWYNG]

fixed in 7ddd20d

[QWYNG]

I have used a Struct to make the contents of action_state more understandable, but using a simple Array would also be good.

[tompng]

In ed_search_prev_history and ed_search_next_history, @prev_action_state == ActionState.new(:search_history, :empty) is working great 👍

In general, I think this kind of ideom will appear frequently.

foobar = @prev_action_state.value if @prev_action_state&.type == :foobar

To make it easy to use, can you add a helper method like this?

# get
prev = prev_action_state(:search_history)
# set
set_next_action_state(:search_history, :empty)

Using it, we can just use a simple array internally. It will reduce total struct/class count (complexity).
And we don't need to think of NullActionState wrongly modified.

state = @prev_action_state
state.value = next_value # bug. this might modify NullActionState
@next_action_state = state

[QWYNG]

Looks good! I tried to represent the state in a 2-element array!
19dae5b

[tompng]

Can you add an initialization of these instance variables in def reset_variables?
This will properly re-initialize just ater Reline.readline is canceled by Ctrl-C.
Initial state of @prev_action_state will be simple. (Currently, initial value is nil and will be updated to NullActionState)

[QWYNG]

Thanks for the advice. I have also added initialization of variables at 19dae5b

[tompng]

Looks good 👍 Thank you!