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

Do not change selection to update lists #145

Merged
merged 12 commits into from
Jan 31, 2024

Conversation

harshad1
Copy link
Collaborator

@harshad1 harshad1 commented Jan 26, 2024

In this PR I have modified several functions to eliminate the need for modifying the current selection or cursor position.

This results in:

  1. Much smaller and cleaner code
  2. Reduced chance that selection is accidentally changed when renumbering etc
  3. Improved performance in some cases

I have also added logic to explicitly save the cursor / selection position and have it restored after the change.

let l:prev_indent = -1
let l:levels = {} " stores all the info about the current outline/list

for l:line in l:selection_lines
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are using just the line numbers anyway so a loop is sufficient

" Reset the starting visual selection
call setpos("'<", [0, a:1[0], a:1[1], 0])
call setpos("'>", [0, a:2[0], a:2[1], 0])
execute 'normal! gv'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't need any of the reset visual selection code

" Iterate the cursor position over each line and then call
" s:change_bullet_level for that cursor position.
call setpos('.', [0, l:lnum, 1, 0])
call s:change_bullet_level(a:direction)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just pass in the number to save on a lot of unnecessary work

if g:bullets_renumber_on_change
" Pass the current visual selection so that it gets reset after
" renumbering the list.
call s:renumber_whole_list(l:start, l:end)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not needed now!

@@ -1089,9 +1075,10 @@ fun! s:get_visual_selection_lines()
let l:index = l:lnum1
let l:lines_with_index = []
for l:line in l:lines
let l:lines_with_index += [{'text': l:line, 'nr': l:index}]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an optimisation.

However, the entire get_visual_selection_lines() is not needed anymore and perhaps ought to be removed.

@harshad1 harshad1 changed the title WIP: Do not change selection to update lists Do not change selection to update lists Jan 29, 2024
" When setting the selection we set the start and end at the same _offset_
" From the new line start and end. As we manipulate line prefixes, the
" offset from the end represents the correct new cursor position
fun! s:get_selection(is_visual)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to pass in is_visual because this state is lost by the time the function is triggered.

@harshad1
Copy link
Collaborator Author

This is ready to merge

Copy link
Member

@dkarter dkarter left a comment

Choose a reason for hiding this comment

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

Thank you!

@dkarter dkarter merged commit 83fa729 into bullets-vim:master Jan 31, 2024
1 check passed
kaymmm added a commit to kaymmm/bullets.nvim that referenced this pull request Feb 18, 2024
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.

None yet

2 participants