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

add option for statusline overwriting #163

Closed
wants to merge 1 commit into from
Closed

add option for statusline overwriting #163

wants to merge 1 commit into from

Conversation

bling
Copy link

@bling bling commented Jul 31, 2013

this would be the easiest route for fixing vim-airline/vim-airline#105. thanks.

@jchain
Copy link

jchain commented Aug 5, 2013

I'm curious. It seems that vim-airline already supports tagbar. Why does this patch still make sense?

@majutsushi
Copy link
Collaborator

I had actually implemented a customisable statusline function similar to what ctrlp does some time ago as an experiment for PowerLine, but since the author didn't have enough time to have a look at it and test it out I haven't merged it yet. I can check whether it still works and push it to a branch if you would like to try it out. That would make it possible to display more information in the Tagbar statusline, like the current sort order.

@bling
Copy link
Author

bling commented Aug 5, 2013

@jchain this patch is specifically for detecting active window splits. tagbar (and a lot of plugins) use eventignore to change windows, so none of the autocmds that airline depends on get fired. i could go into more details but basically to resolve vim-airline/vim-airline#105 it would involve either this patch, a change to the core of tagbar, or a very hacky polling of the statusline value for changes in airline.

@majutsushi
Copy link
Collaborator

By the way, the eventignore is necessary to prevent other plugins like MiniBufExplorer from interfering with window switching, see for example #17 and #56 .

@bling
Copy link
Author

bling commented Aug 5, 2013

@majutsushi yep, understood. btw, is there something you don't like about this change?

@majutsushi
Copy link
Collaborator

Not as such, I just think it would be a bit more useful if the
statusline information wouldn't get lost by overwriting it. In addition
it may be possible to make the CursorMoved autocommand unnecessary (at
least for Tagbar), but I'll have to investigate that a bit more.

@bling
Copy link
Author

bling commented Aug 6, 2013

i just added the use of TagbarGenerateStatusline, so no information should be lost anymore. let me know if you need anything else from me. thanks!

@qva5on3
Copy link

qva5on3 commented Aug 13, 2013

It looks like #165 relates to this issue.

@bling @majutsushi What is currently the best solution for this problem. Should I use bling/vim-airline#105 as workaround. Thanks.

@bling
Copy link
Author

bling commented Aug 13, 2013

@qva5on3 for the original issue in airline, it's been "fixed" since i've resorted to the hack fix. the performance cost is negligible so i included it. if this patch gets merged or if @majutsushi comes up with another solution i'll remove the change from airline.

@majutsushi
Copy link
Collaborator

Hi @bling, I have just pushed some experimental changes to the stltest branch. I have disabled the window switching autocommand ignores (since MiniBufExplorer seems to have been fixed), and added proper flexible statusline functionality like CtrlP.

To use the new statusline assign a function name to g:tagbar_status_func. The function takes three arguments: whether Tagbar is the current window (0 or 1), the sort order as a String, and the filename being displayed (may be empty if no file has been loaded yet). Please let me know if that works for you and if you have any suggestions for improvements!

@bling
Copy link
Author

bling commented Aug 21, 2013

thanks @majutsushi! this is much cleaner. i did some basic tests and it appears to be working. i need some extra work on my side to account for the inactive state, but that was on my todo list anyways so this is a good catalyst. please close off this PR when you feel your changes are good and merged into master. thanks again.

@majutsushi
Copy link
Collaborator

Okay, I've merged the code now. Note that it may be useful to define the statusline function with an additional vararg just so it doesn't break if there's a need to add more arguments in the future.

@majutsushi majutsushi closed this Aug 24, 2013
@bling
Copy link
Author

bling commented Aug 24, 2013

thanks @majutsushi! changes are in on my end as well.

@majutsushi
Copy link
Collaborator

Nice, it seems to work great!

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.

4 participants