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

Bookmarks signs hidden when navigating away from window #118

Open
nkgm opened this issue Apr 5, 2017 · 10 comments
Open

Bookmarks signs hidden when navigating away from window #118

nkgm opened this issue Apr 5, 2017 · 10 comments
Labels

Comments

@nkgm
Copy link
Contributor

nkgm commented Apr 5, 2017

Is there a way for them to remain visible?

@nkgm
Copy link
Contributor Author

nkgm commented Apr 22, 2017

If this is by design, then maybe have an option to keep them visible?

@MattesGroeger
Copy link
Owner

This must be a regression as this was not happening in my original implementation. I'm assuming this happened accidentially as a result of another PR (but maybe there was a reason for that?). To verify my assumption you should find the commit that changed this behaviour using git bisect. Let me know if you need help there.

Also added #120 and #121 which I noticed today during testing. Would be great if you could have a look at these as well. Thanks so much!

@MattesGroeger
Copy link
Owner

If we get the current version polished we can make a new release 👍 – has been a long time since the last release 😉

@nkgm
Copy link
Contributor Author

nkgm commented Apr 24, 2017

Reproduced first in b19ea0a.

The pending CtrlP PR should take care of #120 so, together with this, would be enough to cut a new release. #121 should take a bit longer and would (most likely) come with breaking changes so we're looking at 2.x?

@MattesGroeger
Copy link
Owner

Great find! Any idea already what in the commit changed the behaviour of the gutter icon?

I'm fine with moving the breaking changes to a future 2.x release. 👍

@nkgm
Copy link
Contributor Author

nkgm commented Apr 25, 2017

It's due to the way autosave is implemented. On BufEnter, as part of the autosave/reload dance, bm_sign#del is called for all files clearing all bookmarks, and then bm_sign#add_at is called for the current file only. The said commit merely fixed a buggy if condition (acting as the trigger) as far as I can tell (b19ea0a#diff-da99742eccd8b8573e3f3be5ca8303b0R32), so the bug was probably introduced earlier. I'll study the autosave code a bit more see if it can be improved.

@lcrockett
Copy link

Great and useful plugin, however this bug degrades the usability of vim-bookmarks significantly as you'll have no quick and clear indication of bookmarks in view. This implies you'll have to resort to active bookmark scanning (using telescope-vim-bookmarks.nvim for instance) instead of passive scanning because of sign(s) popping up.

I've tried negating this as much as possible using the configuration (Lua neovim based) below and using BookmarkClearAll + BookmarkLoad 0 upon every BufEnter,BufWinEnter etc. event. This however brings up loading issues with each BookmarkLoad trigger coming up with the error Failed to load/parse file. This is only resolved by switching the active window with another one, and then going back to the original window, negating the effects of the workaround.

Is there any testing we can do to help get this bug fixed ?

vim.g.bookmark_center = 1
vim.g.bookmark_display_annotation = 1
vim.g.bookmark_no_default_key_mappings = 1
vim.g.bookmark_show_toggle_warning = 0
vim.g.bookmark_show_warning = 0

Cheers !

@MattesGroeger
Copy link
Owner

MattesGroeger commented Oct 1, 2021

Hi @lcrockett,

thanks for your comment and support!

I haven't had the chance to investigate the findings from the above comments. If you'd like to help, could you please verify if b19ea0a indeed introduced the issue or if the bug was already present before? Once we know what exactly causes the issue the fix is hopefully easy 🤞

Update: I just tried for myself and indeed this bug is not present with the previous commit 72b3ce8 ... I'll quickly investigate what change exactly causes the glitch

@MattesGroeger
Copy link
Owner

Ok, I hope I found the issue. The fix is up in this PR #169. Please help me testing it 👍

@lcrockett
Copy link

Thank you for the quick fix. Unfortunately this did not change said behaviour. I changed a few autocmd directives in the same changed file of PR #169 (namely BufNew,BufEnter,BufHidden,FocusGained + BufLeave,FocusLost). However that did not fully resolve the issue either.

If you'd like to chase a different approach or need testing for a different fix, let me know. Cheers

JS-Zheng added a commit to JS-Zheng/vim-bookmarks that referenced this issue Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants