Skip to content

Allow granular control over loading pre-processor syntax files #133

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

Merged
merged 3 commits into from
Jul 24, 2019
Merged

Allow granular control over loading pre-processor syntax files #133

merged 3 commits into from
Jul 24, 2019

Conversation

igemnace
Copy link
Contributor

@igemnace igemnace commented Mar 19, 2019

In addition to g:vue_disable_pre_processors, which is an "all or nothing" approach to loading pre-processor syntax files, a g:vue_pre_processors option is exposed as well. This is a List of names of pre-processor syntaxes to load, to allow the user to enjoy syntax highlighting for one or two pre-processor languages without incurring the performance penalty of loading all of them at the same time.

Notable reasons not to merge this:

  • PR Make languages & policies user-configurable #109 does something similar with indent configuration, and acknowledges its possible extension to syntax configuration in this comment. Perhaps a more unified solution can be done instead of merging this PR directly.
  • There is already some work towards loading languages lazily, as in Load languages lazily #128. If this work is completed and the language detection works well enough, there wouldn't be a need for g:vue_pre_processors altogether.

In the meantime, however, as none of the above solutions are fleshed out as of yet (particularly in the context of syntax), this change could be a way to mitigate the issue.

Prior to this commit, there was a single Boolean option,
g:vue_disable_pre_processors, which would only activate either *every*
pre-processor syntax file or none of them at all.

This was a known pain point when it comes to performance. On some
machines, loading all the pre-processor syntax files could slow down Vim
noticeably, hence the need for such an option in the first place.
However, turning all of them off means having to live with no syntax
highlighting at all if one uses a pre-processor language.

This commit introduces another option: g:vue_pre_processors. This is a
List of names of pre-processor syntaxes, e.g. ['pug','scss']. If a user
provides this option, only the named pre-processor syntax files will be
loaded.

This change still allows for g:vue_disable_pre_processors: If
g:vue_disable_pre_processors is truthy, pre-processor syntax files
aren't loaded regardless of the value of g:vue_pre_processors.
The change in commit ba9a3db is
documented in the README, to augment the information regarding
pre-processor languages and g:vue_disable_pre_processors.
@igemnace
Copy link
Contributor Author

Tests are failing. I'll fix shortly.

The plugin's current behavior is reliant on the order of loading
pre-processor syntax files. In particular, Vader tests are failing for
SCSS snippets; SASS syntax identifiers are applied instead.

Unfortunately, no ordering information can be embedded in a Dictionary,
so s:language_config must be turned into a List.

In the unconfigured case (no g:vue_disable_pre_processors, no
g:vue_pre_processors), pre_processors can be initialized to a
well-ordered List of languages (ordered as they appear in
s:language_config), instead of the unstable keys() on a dict.

Random access of language config is faster from a dict, though, so a
dict is rebuilt in s:language_dict. Going through the s:language_config
List for every pre-processor in pre_processors is O(m * n), as opposed
to building s:language_dict once then looping through pre-processors
once for O(m + n) (since dict access is constant time).
janlazo added a commit to janlazo/dotvim8 that referenced this pull request Apr 7, 2019
Anticipating posva/vim-vue#133
which should resolve perf issues with this plugin

coc-vetur provides the autocomplete for Vue SFC.
janlazo added a commit to janlazo/dotvim8 that referenced this pull request Apr 8, 2019
Anticipating posva/vim-vue#133
which should resolve perf issues with this plugin

coc-vetur provides the autocomplete for Vue SFC.
janlazo added a commit to janlazo/dotvim8 that referenced this pull request Apr 8, 2019
Anticipating posva/vim-vue#133
which should resolve perf issues with this plugin

coc-vetur provides the autocomplete for Vue SFC.

Stop coc.nvim from autoupdating coc-vetur on startup
because I run multiple vim/nvim everyday.
I don't use spacemacs because of autoupdate on startup.
janlazo added a commit to janlazo/dotvim8 that referenced this pull request Apr 9, 2019
Anticipating posva/vim-vue#133
which should resolve perf issues with this plugin

coc-vetur provides the autocomplete for Vue SFC.

Stop coc.nvim from autoupdating coc-vetur on startup
because I run multiple vim/nvim everyday.
I don't use spacemacs because of autoupdate on startup.
@janlazo
Copy link

janlazo commented Jul 21, 2019

Is this PR ready?

@adriaanzon
Copy link
Collaborator

Yeah, this should be ready to merge. I have actually been holding off on this because I wanted to see what I could do with #128, but so far I haven't really had the time to work on that. I'll have a further look at this PR tonight and merge it in.

I'm also thinking of merging in the work on the performance-enhancement branch, and enable that when g:vue_pre_processors is set to 'auto' instead of a List. Then, later, its behavior can be enhanced by adding lazy loading described in #128.

@adriaanzon adriaanzon self-assigned this Jul 24, 2019
Copy link
Collaborator

@adriaanzon adriaanzon left a comment

Choose a reason for hiding this comment

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

Looks 👌 to me.

@adriaanzon adriaanzon merged commit 6acff7e into posva:master Jul 24, 2019
adriaanzon pushed a commit that referenced this pull request Jul 24, 2019
It was superseded by g:vue_pre_processors (#133)

I could add a message if g:vue_disable_pre_processors is set, asking
people to update their config to the new variable, but I'd rather not
interrupt people's workflows.
@adriaanzon
Copy link
Collaborator

I'm also thinking of merging in the work on the performance-enhancement branch, and enable that when g:vue_pre_processors is set to 'auto' instead of a List. Then, later, its behavior can be enhanced by adding lazy loading described in #128.

You can now set g:vue_pre_processors to 'detect_on_enter'.

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.

3 participants