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

Fix issue with tagbar changing the global scrolloff value. #694

Merged
merged 2 commits into from Oct 29, 2020

Conversation

raven42
Copy link
Collaborator

@raven42 raven42 commented Oct 29, 2020

Closes #693

Tagbar is overwriting the global scrolloff value which can affect other windows. Change the behavior so it does a setlocal instead when needed so this only impacts the tagbar window

Tagbar is overwriting the global scrolloff value which can affect other windows. Change the behavior so it does a setlocal instead when needed so this only impacts the tagbar window
@raven42 raven42 requested a review from alerque October 29, 2020 16:36
autoload/tagbar.vim Outdated Show resolved Hide resolved
Co-authored-by: Caleb Maclennan <caleb@alerque.com>
@raven42 raven42 requested a review from alerque October 29, 2020 18:02
@alerque alerque merged commit 30b20fc into preservim:master Oct 29, 2020
@Shane-XB-Qian
Copy link
Contributor

looks this change did not work well, e.g set scrolloff=0 at line#2126
did not dig code detail, just chk looks it seems like that .. and influenced others windows if set value to this option.

@raven42
Copy link
Collaborator Author

raven42 commented Oct 30, 2020

@Shane-XB-Qian can you please elaborate? This change was to update tagbar to use setlocal instead of set so it is not changing a global setting.

@Shane-XB-Qian
Copy link
Contributor

sorry, i have no time to dig detail, but for now obviously not correct:
you can e.g start vim with let g:tagbar_scrolloff=16 and set so=10, you can see globally value had been influenced.

@raven42
Copy link
Collaborator Author

raven42 commented Oct 30, 2020

@Shane-XB-Qian I think you must have not pulled in this fix. This fix specifically fixes the issue you are referring to. The g:tagbar_scrolloff field was introduced in #692. But this fix was to correct the behavior which already existed in the tagbar code where it was setting the global scrolloff. Please make sure you pull in the latest and try again. I tested with the values you specified and when I issue a echo &scrolloff in the tagbar window, I see 16, and then without changing anything if I issue that in another window, I see a value of 10.

@Shane-XB-Qian
Copy link
Contributor

// I got upt 13hours ago, your latest pr was 23hours ago.
What this let &l:scrolloff = scrolloff_save for?
And check line#2126 there is set scrolloff=0 too.
I felt a little mess now, and not sure why you can get 10.
I felt those code original author was trying to save &so value, then zt, then recover.
Anyway, I am not near my computer now, wish you can review/check again.

@alerque
Copy link
Member

alerque commented Oct 30, 2020

What this let &l:scrolloff = scrolloff_save for?

That sets the buffer local scroll off value to the previously saved value.

And check line#2126 there is set scrolloff=0 too.

This is a second instance of the problem we were fixing. This PR fixed one instance, that is a different one.

I felt those code original author was trying to save &so value, then zt, then recover.

The original code was saving a value, setting it to something else, doing the action, then restoring the value. The problem was the set it was doing was a global set instead of a local one, then restored it also by doing a global set instead of local.

@raven42 The instance you fixed is in s:RenderContent(), the one tripping up @Shane-XB-Qian is another instance of the same kind of error in s:RenderKeepView() that we didn't touch at all.

@raven42
Copy link
Collaborator Author

raven42 commented Oct 30, 2020

Honestly I'm not sure the purpose of this little block of code. I found this while looking at another issue and thought I'd fix it so we aren't changing the global scrolloff value.

@raven42
Copy link
Collaborator Author

raven42 commented Oct 30, 2020

I've pushed up another PR (#697) that will address the other instance of this bug. I've also looked for any other instances where the scrolloff is being adjusted, and this looks like the only other place.

@raven42 raven42 deleted the scrolloff-local-fix branch January 6, 2021 13:50
dev-hann added a commit to dev-hann/tagbar that referenced this pull request Sep 18, 2023
…#694)

Co-authored-by: Caleb Maclennan <caleb@alerque.com>
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.

tagbar is changing global scrolloff value
3 participants