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

A simple way to implement tagbar async call #561

Closed
wants to merge 1 commit into from
Closed

A simple way to implement tagbar async call #561

wants to merge 1 commit into from

Conversation

pangchol
Copy link

@pangchol pangchol commented Oct 30, 2019

Closes #532.

Copy link
Member

@alerque alerque left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to contribute @pangchol. I've made a few notes of things that will need refactoring before this can be included. There might be more, but starting with those should make it a lot easier to review what is left. Note also the comment message should be en English, and don't forget to run the liter as that will be required. You can see the output here from the PR or run it locally yourself with vint .. Thanks!

autoload/tagbar.vim Outdated Show resolved Hide resolved
let g:tagbar_update_fname = a:fname
let g:tagbar_update_force = a:force
let g:tagbar_no_display = a:0 > 0 ? a:1 : 0
if has('win32') " windows use timer_start will call bug
Copy link
Member

Choose a reason for hiding this comment

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

Where is this bug documented upstream (so we can track when to revert workarounds)

let no_display = a:0 > 0 ? a:1 : 0
function! AutoUpdate_CB(channel, ...) abort
" call tagbar#debug#log('AutoUpdate called [' . fname . ']')
let l:fname = g:tagbar_update_fname
Copy link
Member

Choose a reason for hiding this comment

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

Why are you passing global variables around here? This seems to rely on side effects, this function should be refactored to accept all the arguments it will use (as in the function it is replacing).

Copy link
Author

Choose a reason for hiding this comment

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

The use of global variables is because I don't know how to pass parameters in a callback , so I'm just using global variables for simple pass paramenters.

Copy link
Member

Choose a reason for hiding this comment

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

We're going to have to figure out how to do that, or at least some other mechanism which will protect them from being clobbered. As soon as asynchronous jobs start getting involved it's really easy to introduce bugs if you are relying on side effects. I don't want to start down that road here.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like you can pass a function reference, so something like:

call timer_start(1, function('AutoUpdato_CB', [a:jobid], fname))

But you'll have to play around with specifics. The async.vim code might give you some clues.

Copy link
Collaborator

Choose a reason for hiding this comment

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

so after some experimentation, the timer_start() routine operates in the same thread context. So this would only delay the processing of ctags, and any apparent 'lockup' effect of vim during ctags processing will still occur, only a little bit after once the timer pops.

plugin/tagbar.vim Outdated Show resolved Hide resolved
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

suggest let s:AutoUpdate function run async
3 participants