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

Some refactor on hop #58

Merged
merged 16 commits into from
Nov 8, 2023
Merged

Some refactor on hop #58

merged 16 commits into from
Nov 8, 2023

Conversation

yehuohan
Copy link

@yehuohan yehuohan commented Nov 5, 2023

Hop has beautiful and readable code with comment string for types. And this PR is to make some refactor to make hop better:

  • add WindowRow,WindowCol,WindowCell and WindowChar to make hop position computation less confused
  • make hop-treesitter and hop-yank as hop extensions according to hop framework
  • add jump_regex.lua to make jump_target.lua thin
  • add jump_target.move_jump_target to compute hint_offset (according to hop's comment, I think hint_offset wants to make offset by visible cells)
  • add hop_helpers.lua to provide test functions for hop
  • refactor: compute unmatched area with LineRange and ColumnRange
  • refactor: remove MatchContext and make JumpContext thin (I think complete JumpContext and Options passed to Regex.match is better for hop-extension developement)
  • fix: take the first folded line as an empty line, so we can jump to the first column of folded block too
  • refactor: add clip_line_context and make jump_targets_by_scanning_lines thin
  • refactor: add jump_target_generator to make creating Locations more easily and provide hop-extension ability to produce custom WindowContext[]

This is not a feature PR, but is to provide more typed and readable code instead.
@smoka7 Are you willing to review this PR?

@smoka7
Copy link
Owner

smoka7 commented Nov 5, 2023

Hi! I had a quick look and it seems very good.All your proposed changes improve the code base and made it more consistent. Well done 👍.

Only problem is dimming unmatched lines is broken. for example :HopWordBC dims the line below the cursor.
it should be tested in

  • empty lines
  • first and last line of buffers
  • first and last columns of a line
  • include the cursor when dimming after or before cursor

@yehuohan
Copy link
Author

yehuohan commented Nov 6, 2023

@smoka7 My careless for dimming area. I refactored dimming related code, and now clip_window_context will give the correct area for advanced operations.

By the way, do you have any idea about what's the purpose of the add_virt_cur? I just have a better displayed cursor without add_virt_cur. (I tested on neovim-qt, neovide and nvy)

With add_virt_cur, sometime I got a double-cursor displayed:
图片
Or a dark cursor:
图片

@smoka7
Copy link
Owner

smoka7 commented Nov 6, 2023

By the way, do you have any idea about what's the purpose of the add_virt_cur? I just have a better displayed cursor without add_virt_cur. (I tested on neovim-qt, neovide and nvy)

Nvim hides the cursor when it waits for user input while running getcharstr . The point of add_virt_cur is curing this. I've seen other plugins patch getcharstr to do similar thing to hop but I can think of any reason of its usefulness.

With add_virt_cur, sometime I got a double-cursor displayed:

Yes it happens on empty lines, it might be an Nvim issue. I think it's better to make it optional with default to false since it has a buggy behavior.

@smoka7 smoka7 linked an issue Nov 6, 2023 that may be closed by this pull request
@smoka7 smoka7 linked an issue Nov 6, 2023 that may be closed by this pull request
@smoka7 smoka7 merged commit df0b5b6 into smoka7:master Nov 8, 2023
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.

HopLine no longer labels folds Jump back (i.e. ctrl-o) is broken after hop (at least :HopChar1)
2 participants