Continuation of psliwka/vim-smoothie#21

[subnut]

Closes psliwka/vim-smoothie#1 Closes psliwka/vim-smoothie#11 Closes psliwka/vim-smoothie#12 psliwka/vim-smoothie#17 Closes psliwka/vim-smoothie#18 psliwka/vim-smoothie#19 Closes psliwka/vim-smoothie#20 Closes psliwka/vim-smoothie#22

[subnut]

@psliwka Please review

[psliwka]

Thanks! Commits up to 24b7868 LGTM, so I've merged them partially to the master to let our users start benefiting from them, and also give us some sense of progress here :D

In the process, I took the liberty of rewording 24b7868, to prevent it from closing #17 and #19 upon merge (they shouldn't be closed yet, as the fix is partial). Furthermore, I backported the ^F-related bugfix from 72c05fb4ef475328716e9e1ec66ecb4f88c686c3. This will give you a slight conflict when rebasing onto newest master – sorry about that.

[psliwka]

Now, let's talk about gg and G. They are completely broken here in #23, because they rely on g:smoothie_disabled which was reworked into g:smoothie_enabled since #21.

There is also another bug I've discovered when playing with these mappings (from #21): using a line number larger than total count in the file causes smoothened gg and G to scroll to the bottom then freeze vim indefinitely.

[subnut]

You are correct! I shall fix them ASAP.

[subnut]

@psliwka I rebased from latest master, and then I went back and amended the respective commits where gg and G were implemented. Please check whether the issues have been resolved or not.

[subnut]

prevent it from closing #17 and #19 upon merge (they shouldn't be closed yet, as the fix is partial).

@psliwka I think #17 should be closed, because OP was only looking for a way to disable the plugin -

I am now looking for a way to disable this plugin in all firenvim instances.

which can be done using g:smoothie_enabled like so -

if exists('g:started_by_firenvim')
    let g:smoothie_enabled = 0
endif

[subnut]

I agree #19 should stay open for a more generic (automatic, if possible) fix

[MasterMedo]

Now, let's talk about gg and G. They are completely broken here in #23, because they rely on g:smoothie_disabled which was reworked into g:smoothie_enabled since #21.

There is also another bug I've discovered when playing with these mappings (from #21): using a line number larger than total count in the file causes smoothened gg and G to scroll to the bottom then freeze vim indefinitely.

For me the G didn't freeze nvim on its own, but if there's a fold on the bottom line that I try opening then it freezes. Although, ctrl-c does unfreeze it. For the past couple of days I had vim-smoothie with gg and G. I've got to say I loved every second of it! Being able to quickly scroll through the whole file gives you enough time to identify which file is it that you're looking at. I found myself doing G, gg upon opening new files. Just my two cents!

[subnut]

@MasterMedo I had not considered the fact that there may be a fold on the last line. Sorry.

Thanks for reporting the issue. I shall fix it ASAP.

[subnut]

@MasterMedo Phew! That was a HUGE loophole in how folds are handled! Thank you! The issue should be fixed in the latest commit. Please check.

Thanks again for reporting bug.

[MasterMedo]

Now I'm experiencing the bug @psliwka reported. Pressing G freezes the file. Open with nvim -u init.vim init.vim

init.vim:

call plug#begin('~/.vim/plugged')
Plug 'subnut/vim-smoothie'
call plug#end()
noremap <silent> <Plug>(Smoothie_G) <cmd>call smoothie#G()<CR>
silent! map <unique> G <Plug>(Smoothie_G)
" {{{
" vim: fdm=marker
" }}}

EDIT: one should replace Plug 'subnut/vim-smoothie' with Plug 'subnut/vim-smoothie', { 'branch': 'devel' }

[subnut]

@MasterMedo Please check again after :PlugUpdate vim-smoothie | PlugClean | PlugInstall vim-smoothie. I am unable to reproduce at 77baa0

[MasterMedo]

That wasn't the issue, you changed the branch to devel, I didn't notice. That is fixed now. But now I have another problem. Wrapped lines aren't treated as separate lines anymore, but as a one big line so it just teleports me down when scrolling.

EDIT: actually, was that ever a feature or am I misremembering, maybe I should open a separate issue for that as an enhancement?

EDIT2: actually, that was never a feature, I'll open a separate issue for that as an enhancement.

[benwoodward]

Fantastic work guys. I've discovered a small issue, which is that if you hit gg to scroll to the top, and hit G or ctrl-d or any other scrolling hotkey, it's not possible to interrupt the scroll, and what happens is that the different scroll events (scroll to top, scroll to bottom, page down etc) will happen in sequence. If you hit a bunch in a row you have to wait for Vim to stop scrolling around before you can work again. I guess this is more of an enhancement request than a bug.

[subnut]

@benwoodward See point 2 of https://github.com/psliwka/vim-smoothie/issues/1#issuecomment-560158642

2. Cursor movement commands are frequently used to operate on ranges, f.ex. gggqG. The plugin will need to detect such cases, and either skip the animation, or delay executing further commands until the animation ends. Otherwise, subsequent commands would be applied to incorrect ranges.

[benwoodward]

@benwoodward See point 2 of #1 (comment)

1. Cursor movement commands are frequently used to operate on ranges, f.ex. gggqG. The plugin will need to detect such cases, and either skip the animation, or delay executing further commands until the animation ends. Otherwise, subsequent commands would be applied to incorrect ranges.

Oops sorry, didn't read closely enough.

[subnut]

@MasterMedo I think you are probably misremembering, but you can always revert to Plug 'psliwka/vim-smoothie' and check it for yourself.

[BStephenBB]

I made an issue yesterday, but just saw this PR which seems to be making a whole bunch of different improvements. Does anyone know if the fixes/improvements in this PR will address #24?

[subnut]

24 has been solved

[MasterMedo]

G still freezes nvim when G is invoked directly after closing a fold, and other odd behaviour which I wasn't able to reproduce in a minimal vimrc but I'll see if they go away after this is fixed.

init.vim:

" vim: fdl=1 fdm=marker
call plug#begin('~/.vim/plugged')
Plug 'subnut/vim-smoothie', { 'branch': 'devel' }
call plug#end()
silent! map <unique> G  <Plug>(Smoothie_G)

" {{{
"
"
"
"
" }}}

Open with nvim -u init.vim init.vim and do 9G, zc, G and it'll freeze.

[subnut]

@MasterMedo Try the latest commit. It should (theoretically) fix the problem of folds for G and gg forever.

[MasterMedo]

I don't think this should be considered 'solved', now it's so slow it's unusable. A folded line should be treated as one logical line for the animation.

Consider a similar init.vim as before;

" vim: fdl=0 fdm=marker
call plug#begin('~/.vim/plugged')
Plug 'subnut/vim-smoothie', { 'branch': 'devel' }
call plug#end()
silent! map <unique> G  <Plug>(Smoothie_G)

" {{{
" duplicate this line 10000 times
" }}}
" last line

except that amount of the lines in the fold is ~10000 or more. One will wait quite a bit on each fold unnecessarily. Here's what I'm thinking (in pseudocode):

while True:
    if current_line_number is closed_fold:
        current_line_number = get_end_of_fold(current_line_number)

    current_line_number = current_line_number + 1

The idea is to skip animating the lines in closed folds but perform an action if one is issued. Mind you, I have no idea if this is possible, partial animation sounds like something that should be well tested before merging.

Also, what about animating only if no command is issued, meaning a raw G or gg, I see this is partially the case already, but I'd like it be expanded to {number}{motion}, and limit animating to normal mode. If this won't be supported one is forced to map this function to a key other than G, like <leader>G, which is reasonable.

The more I think about it the less I like the complexity. Aren't we reinventing the wheel here? Can't one just hold ctrl-d?

[subnut]

@MasterMedo Thanks for that pseudo-code! That helped a lot! See the latest commit for the translation of that pseudo-code to vimscript-code :wink:

It should fix all fold issues, theoretically

[MasterMedo]

Happy to help ^^, I'll report if I encounter more unwanted behaviour.

[subnut]

Ping @psliwka

[psliwka]

On it!

[psliwka]

Alright, so far I've merged gg and G into master. Thanks everyone involved in bugfixing these beasts 💪 Additional notes:

1. I agree these commands are not safe enough to be enabled by default. However, I've made the opt-in process a little bit easier with a0414d1203e55d21b2b81050173c1c76bf0f854f.
2. I don't like the comment removal idea introduced in a37f3d11db8eb4d8e486bd6bd1536692b7d0978c. The code is indeed heavily duplicated at this point, but I strongly believe the correct way to proceed here is to follow DRY and abstract the repeating parts into dedicated functions. This will naturally remove duplicated comments, and additionally will help in further development and maintenance.

I'll review the rest of changes made here as soon as I find a next time slot (deadline: EOW).

[subnut]

2. I don't like the comment removal idea introduced in a37f3d1. The code is indeed heavily duplicated at this point, but I strongly believe the correct way to proceed here is to follow DRY and abstract the repeating parts into dedicated functions. This will naturally remove duplicated comments, and additionally will help in further development and maintenance

Done! See the latest implementation of cursor-movements! It's amazing!

I also rebased from master to help you merge this PR instantly (if you wish to)

[psliwka]

See the latest implementation of cursor-movements! It's amazing!

Awesome indeed. It's in! 💪

[psliwka]

21541f2bff7be64cfd03723798975d23c6c4d286 is being deprecated by 2c490c2a40f5c018b13b150cbcfc866dadb448aa, which consolidates contributors list in one place, and makes it easier to add new contributors in the future.

[psliwka]

The #18 fix is in. It seems to tear the animation smoothness a bit when nonzero 'scrolloff' is used, but that's still better than getting stuck completely.

[psliwka]

The cursor velocity curve tweak (0e3c8e4776e35a7cad264bdd9f783fd2e936007a) has been superseded by 6cd5dea1698c0238a6d08eb8fc66696007483640 and 4bb75e5f85f933617cf1328d8a0089f85e8100c4. Kudos for the idea of using power function!

[psliwka]

210215f21c6228a1c95ea20922285c1f1c020046 has been sort-of-superseded by 10fd0aa57d176718bc2c570f121ab523c4429a25. Remaining parameters will be added to the README as they mature and stabilize.

[psliwka]

Alright, looks like all the work here has been incorporated to the master, so I'm marking this as resolved. Thanks again!