You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The plugin is detecting Darwin by system uname command. But the system() call is too slow, and I noticed it makes the starting method slow, also. In this PR, has('macunix') is used for detecting Darwin instead of system() call.
You can see this with +profile feature.
vim --cmd "profile start profile.txt" --cmd "profile file /path/to/plugin/webdevicons.vim" +q && vim profile.txt
SCRIPT /path/to/plugin/webdevicons.vim
Sourced 1 time
Total time: 0.286557
Self time: 0.001238count total (s) self (s)
" Version: 0.7.0" Webpage: https://github.com/ryanoasis/vim-devicons" Maintainer: Ryan McIntyre <ryanoasis@gmail.com>" License: see LICENSE10.000003lets:version='0.7.0'10.2848130.000301lets:operatingsystem=system("uname -s")
" standard fix/safety: line continuation (avoiding side effects) {{{1"========================================================================...
Mac OS X 10.11.2
Vim 7.4.972 (MacVim snapshot 87)
Vim took 0.286557 second with its loading, and system("uname -s") occupied most of it, 0.284813 second.
Sorry, I was mistaken. has('mac') or has('macunix') means it to be built as MacVim. So both are false in a normal vim such as /usr/bin/vim even if on OS X Darwin.
After all, we have to call uname for detecting OS correctly, but it took a bit time. The starting time of my Vim doubled with this plugin. My suggestion for detecting OS logic is below...
If has('macunix'), it is Darwin.
If not has('macunix') and has('unix'), call uname -r.
If output is Darwin, it is Darwin. If not, it is other unix system such as Linux.
system() call will take a bit time. If you want to avoid this, you should set a variable (ex. g:devicons_operating_system) to detect OS without calling system().
Sorry for long delay. Overall I like this change but have some questions and feedback.
The 2nd conditional seems a little odd to me. Couldn't it be eliminated if g:WebDevIconsUseSystemCallToDetectOS was used in the third conditional.
I think by default the system call should be done if we still aren't 100% from the info from feature-list
Maybe instead of g:WebDevIconsUseSystemCallToDetectOS being an on|off it could be a string to allow the user to just specify the system they are on and thus even avoid the system call when it is set. With this I think the system call will only fire if: unix file format and not mac vim and the variable not set. Also we could move the detection call so it doesn't fire for Dos fileformat. Thoughts?
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The plugin is detecting Darwin by system
uname
command. But thesystem()
call is too slow, and I noticed it makes the starting method slow, also. In this PR,has('macunix')
is used for detecting Darwin instead ofsystem()
call.You can see this with
+profile
feature.Vim took 0.286557 second with its loading, and
system("uname -s")
occupied most of it, 0.284813 second.