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

LineEditor Re-architecture #595

Closed
wants to merge 29 commits into from
Closed

Conversation

tompng
Copy link
Member

@tompng tompng commented Oct 24, 2023

Description

This pull request is a refactor of line_editor.
It also fixes many rendering, cursor position, internal state inconsistency bugs including #581 #575 #520 #491 and more, especially when input lines > screen height.

Problem of LineEditor

  • Too many duplicated state, causing inconsistency between several state
  • Key action modifies cursor position
  • Key action controls where to rerender
  • Key action sets actions to be handled in rendering methods
  • Rendering is not idempotent. It also modifies state.
  • Does not have differential rendering mechanism, making rendering complex.

What we should do is something like "jQuery to React"

Changes

Single source of truth - Remove duplicated state.

Important state are

  • Input: @buffer_of_lines
  • Cursor position: @line_index @byte_pointer
  • Scroll offset: @scroll_partial_screen
  • Screen size: @screen_size
  • Reline-controlled line starts from: @cursor_base_y
  • Current cursor y in screen: @cursor_base_y + @cursor_y

Removed these because it can be calculated
@line @highest_in_all @highest_in_this @screen_height
@cursor @cursor_max @first_line_started_from @started_from
@rest_height

Removed these because it is not a state but something like an action dispatch and a flag for rendering
@previous_line_index @just_cursor_moving @last_key
@add_newline_to_end_of_buffer @prev_mode_string @rerender_all
@move_up @check_new_auto_indent

Removed these because rendering is improved.
@cached_prompt_list @prompt_cache_time: no need to cache with time anymore.
@previous_rendered_dialog_y: not needed anymore because dialog rendering strategy is changed.

Render only from state

Don't render, don't move cursor except from single rendering method render_differential.

Lifecycle is simplified

loop do
  # Handles action caused by key input, update dialog content. does not render or move cursor
  input_keys.each { |c| @line_editor.update(c) }
  # Renders from state. Does not change state. Idempotent operation.
  @line_editor.rerender
end 

Differential rendering

render_differential calls render_line_differential(old_items, new_items) for each line.
Input line and overlay dialogs are abstracted as set of [x, width, content].

prompt> code[DIAL[dialog]DIALOG] is represented as
old_items = [[0, 16, "prompt> codecode"], [12, 20, "[DIALOGDIALOGDIALOG]"], [17, 8, "[dialog]"]]

prompt> codecode [dialog] is represented as
new_items = [[0, 16, "prompt> codecode"], nil, [17, 8, "[dialog]"]]

Calculate z-index for each column

prompt> code[DIAL[dialog]DIALOG]
00000000000011111222222221111111

prompt> codecode [dialog]
0000000000000000 22222222

Compare z-indexes and extract where to update
00000000000011111222222221111111
0000000000000000 22222222
------------0000B-------- # ('-' represents unchanged, 'B' represents blank)

Need to render "code " at column 12 and clear after column 25

@tompng
Copy link
Member Author

tompng commented Dec 1, 2023

Released this change as reline-0.5.0.pre.1

https://rubygems.org/gems/reline/versions/0.5.0.pre.1
https://github.com/ruby/reline/releases/tag/v0.5.0.pre.1

Changes and fix to 0.5.0.pre.x will be pushed to #614

@tompng tompng closed this Dec 1, 2023
@tompng tompng deleted the differential_rendering branch March 24, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants