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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent window view when exiting Bufstop window #24

Merged
merged 7 commits into from Aug 10, 2021

Conversation

bruhtus
Copy link
Contributor

@bruhtus bruhtus commented Aug 8, 2021

I have noticed that sometimes the current line position got changed when exit Bufstop window. Sometimes the current line position at the top (like doing normal mode command zt) and sometime the current line position in the middle (like using normal mode command zz). That's kind of annoying, so I tried to use winsaveview() to make window view (current line position, etc) more consistent when exit Bufstop window.

I haven't tested it in split window, but I think it should be the same.

My remove trailling whitespace autocommand kicks in and that makes it looks like I changed a bunch of line, so please ignore that 馃槄

To decrease startuptime, it's better to load only a few necessary
function, in this case is `s:BufstopAppend()`,
`s:BufstopGlobalAppend()`, and `s:TimeoutFiddle()` function to append
bufstop history. Other function can be called using autoload function.
Sometimes when exit Bufstop window, the current line position changed
unpredictably (sometimes the current line moved to the top and sometimes
stay in the middle). Using `winsaveview()` can improve consistency of
current line position.
@mihaifm
Copy link
Owner

mihaifm commented Aug 8, 2021

This is very promising if it works. I didn't know about winsaveview, guess there is something to learn every day. This issue bothers me too, but kind of learned to live with it. Let me test this, it would be a very useful fix.

@bruhtus
Copy link
Contributor Author

bruhtus commented Aug 8, 2021

I didn't know about winsaveview, guess there is something to learn every day.

I didn't realize at first that I could initialize winsaveview() at the command until I saw fzf.vim source code and I thought "maybe it can solve Bufstop current line position issue?" and then here we are.

Let me test this, it would be a very useful fix.

Alright, let me know if there's some problem. I only apply winsaveview() in Bufstop, BufstopPreview, and BufstopFast because I thought it has something to do with Bufstop window, but if you think we should apply winsaveview() in every Bufstop command then I'll change the script again.

EDIT:
I forgot to remove extra g:BufstopData variable 馃槄

Comment on lines +191 to +192
endif
endfunction
Copy link
Owner

Choose a reason for hiding this comment

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

You need to return to the previous window here (wincmd p), othewise your code won't work when two or more windows are present (split windows).

Copy link
Contributor Author

@bruhtus bruhtus Aug 9, 2021

Choose a reason for hiding this comment

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

Ah I see, between endif and endfunction right? for single window (not using split window), there's no problem?

EDIT:
Wait, I think it's more appropriate to put wincmd p between q and if, it will look like this:

q
wincmd p
if exists('b:bufstop_winview')
  call winrestview(b:bufstop_winview)
  unlet b:bufstop_winview
endif
endfunction

go back to previous window first, and then apply winsaveview()

Copy link
Owner

Choose a reason for hiding this comment

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

yes, this should work

@bruhtus
Copy link
Contributor Author

bruhtus commented Aug 10, 2021

I think I found another issue, when deleting the first buffer (6612881), the window view changed back to how it was before (the current line position at the top like doing zt normal mode command).
I'm not sure where I should initialize the winsaveview() when delete the buffer, do you have any idea where should I initialize the winsaveview()?

Inside

if delkey
   call s:BufstopWipeBuffer(s:bufnr)

or inside

function! s:BufstopWipeBuffer(bufnr)

or maybe somewhere else?

EDIT:
I think the dummy buffer causing the window view not restored when exiting Bufstop window.

Here's a step to reproduce that:

  • open a file with vim
  • put the cursor at the bottom of the window by doing L normal mode command
  • open another file (using a buffer, not split window)
  • delete the recent opened file with Bufstop window
  • and then close Bufstop window using BufstopDismissKey

EDIT 2:
When I delete the first buffer and it back to the second buffer, the second buffer doesn't have b:bufstop_winview variable even tho it is in opened.

I think I figure out where I should initialize winsaveview(), It's in s:BufstopWipeBuffer(bufnr) function, like this:

exe window . "wincmd w"
exe "silent b" candidate
if !exists('b:bufstop_winview')
  let b:bufstop_winview = winsaveview()
endif

CMIIW

@mihaifm
Copy link
Owner

mihaifm commented Aug 10, 2021

I think I figure out where should I initialize winsaveview(), It's in s:BufstopWipeBuffer(bufnr) function, like this:

exe window . "wincmd w"
exe "silent b" candidate
if !exists('b:bufstop_winview')
  let b:bufstop_winview = winsaveview()
endif

This looks good

Repository owner deleted a comment from bruhtus Aug 10, 2021
Also initialize `b:bufstop_winview` variable in second buffer if the
first buffer in Bufstop history deleted.
@mihaifm mihaifm merged commit ae6aed2 into mihaifm:master Aug 10, 2021
@mihaifm
Copy link
Owner

mihaifm commented Aug 10, 2021

Good stuff, thanks for the contribution, it's an awesome fix. I will do some minor refactoring on top of these changes.

@mihaifm mihaifm changed the title Consistent window view when exits Bufstop window Consistent window view when exiting Bufstop window Aug 10, 2021
@bruhtus
Copy link
Contributor Author

bruhtus commented Aug 10, 2021

Glad I could help!

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