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

Autoload plugin initialization functions on demand #238

Closed
wants to merge 1 commit into from
Closed

Autoload plugin initialization functions on demand #238

wants to merge 1 commit into from

Conversation

kutsan
Copy link
Contributor

@kutsan kutsan commented Mar 15, 2018

I don't use any of these plugins and current codebase loads them everytime with no reason, it slows things down in a scale. Using autoload functions and loading them on demand is superior IMO.

If it's possible we should use more autoload functions FYI.

@ryanoasis
Copy link
Owner

Wow you've been busy eh? 😉

I have been aware this is a best practice and I'd be all for changing things to be better/faster! 😄

Do you have any metrics or benchmarks just curious

@kutsan
Copy link
Contributor Author

kutsan commented Mar 16, 2018

Honestly, I'm not really sure how to benchmark this. There is no obvious performance boost but if vim docs are true, it should be faster.

@ryanoasis ryanoasis added this to the v0.11.0 milestone May 3, 2018
@ryanoasis
Copy link
Owner

@kutsan Sorry for the late follow up. Is this fork no longer existing (I see unknown repository)?

I can do it via the patch method I suppose, hopefully it will mark the PR as merged.

@kutsan
Copy link
Contributor Author

kutsan commented Feb 6, 2019

@ryanoasis Yeah, sorry about that, I'm not sure why I deleted it.

@ryanoasis
Copy link
Owner

@kutsan No problem thanks. Was just curious. I'll get it merged in

@ryanoasis
Copy link
Owner

@kutsan This looks really good except for one issue I caught while testing a local branch with the changes.

Because of the move to the autoload script files there was a local variable that was now referenced that only existed in the plugin script file.

I was getting this error (and following of course ctrlp plugin integration was breaking):

Error detected while processing function <SNR>97_initialize[9]..devicons#plugins#ctrlp#init:
line    2:
E121: Undefined variable: s:plugin_home
E15: Invalid expression: s:plugin_home . '/status_warned_ctrlp'

However I think I have it fixed with these changes to your PR: https://gist.github.com/ryanoasis/4e1bdf027580cbd4d2b89622dcaeec91

@kutsan
Copy link
Contributor Author

kutsan commented Feb 10, 2019

Yeah, your changes looks okay to me.

@ryanoasis
Copy link
Owner

This has been merged!

Note: This PR also didn't get correctly marked as merged even though it is. Maybe because the fork was removed. Going to mark as closed instead for now

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.

2 participants