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

feat: support JavaScript syntax on template #150

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

vhoyer
Copy link

@vhoyer vhoyer commented Aug 15, 2020

This support:

  1. Non-zero length dynamic values (:attr="variable");
  2. Zero length dynamic values (:attr="");
  3. Slots syntax (#name) now have JavaScript syntax also (#default="{ thisHere }");
  4. Dynamic values with a string right at the begining and the end (v-text="'foo'");
  5. Dynamic keys (:[thisHere]="{ thisAlso: 1 }");

There is also comments explaining commands and the mustache syntax is
considered htmlSpecialChar, because it's easier to see them that way.

Closes #143

This support:
1. Non-zero length dynamic values (:attr="variable");
2. Zero length dynamic values (:attr="");
3. Slots syntax (#name) now have JavaScript syntax also (#default="{ thisHere }");

There is also comments explaining commands and the mustache syntax is
considered htmlSpecialChar, because it's easier to see them that way.

Closes posva#143
@vhoyer
Copy link
Author

vhoyer commented Aug 17, 2020

Oh wait, this has a bug 😅

<img
  :alt="'foo bar baz'"
>

this is breaking the syntax 🤔 donno how to fix, I wouldn't merge this right now

@vhoyer
Copy link
Author

vhoyer commented Aug 17, 2020

Fixed, we can merge again 😅 😂

@vhoyer
Copy link
Author

vhoyer commented Oct 16, 2020

So I've been using it for a while and haven't seem anymore errors

@adriaanzon adriaanzon self-assigned this Oct 19, 2020
@vhoyer
Copy link
Author

vhoyer commented Oct 19, 2020

image
here is an example image of the syntax :D

P.S.: As a side note for the future, I'd like to state that this is not an example of how I code, just a example of the various ways the syntax might show up mashed up in one example :D 😅

@adriaanzon
Copy link
Collaborator

Hi! First of all, thanks for all the effort! Looks very promising.

I took some time to try it out with a barebones vim config (including othree/html5.vim and pangloss/vim-javascript) but it didn't seem to fully work for me.

image

In this basic example, the class attribute binding doesn't get highlighted as JavaScript. Can you see what is going wrong here?

@vhoyer
Copy link
Author

vhoyer commented Nov 13, 2020

hey, I'm investigating this and I discovered that if you insert in your .vimrc also the line:

let g:vue_pre_processors = []

The hightlighing begins to work, but now my lunch break is over, so I will continue investigating this asap 😄 thanks for the feedback ❤️

@vhoyer
Copy link
Author

vhoyer commented Nov 20, 2020

Other discovery is that: having

      \ {'name': 'pug',        'tag': 'template', 'attr_pattern': s:attr('lang', '\%(pug\|jade\)')},
      \ {'name': 'slm',        'tag': 'template'},
      \ {'name': 'handlebars', 'tag': 'template'},
      \ {'name': 'haml',       'tag': 'template'}, " <<<<- this here
      \ {'name': 'typescript', 'tag': 'script', 'attr_pattern': '\%(lang=\("\|''\)[^\1]*\(ts\|typescript\)[^\1]*\1\|ts\)'},
      \ {'name': 'coffee',     'tag': 'script'},
      \ {'name': 'stylus',     'tag': 'style'},

source

breaks the syntax 🤔

when I remove the haml line, it works, very strange...

@adriaanzon
Copy link
Collaborator

Interesting, I also remember having to change the order of that array in order to make some syntax highlighting work.

I can confirm that it works when I set let g:vue_pre_processors = 'detect_on_enter', which is what I would normally use.

Maybe it's reasonable to use 'detect_on_enter' as a default (related: #128). The drawback is that it doesn't highlight new preprocessors as you type, but I think that's fair. When Vim detects shell scripts without file extension, you also have to manually reload the buffer using :e after setting the shell in the shebang (#!) line. Changing to 'detect_on_enter' will also allow us to add more languages (from pending PRs) without slowing down the initial load time.

@vhoyer
Copy link
Author

vhoyer commented Nov 21, 2020

Yeah, I guess I do agree with you regarding the "detect_on_enter" on default.

But the problem happens when the haml syntax loads. When you use "detect_on_enter", do you use haml? (Because I don't, that's why I hadn't seen the bug before)

@vhoyer
Copy link
Author

vhoyer commented Dec 28, 2020

Ok, I'm thinking about it here, I have the smallest clue about using haml and a even smaller one about using haml with vue, but for what I could see in some examples here on github... I don't think the code I made even applies anyway, because the syntax is really different, in fact I think this same logic applies to the other langs for template too, so what I'm thinking here is, that I make adjustments to this PR to make it only work in template without any other lang loaded into it, what do you think, @adriaanzon ?

@adriaanzon
Copy link
Collaborator

Yeah that seems like a good solution to me.

If we ever want to add support for this to other templating languages, we will probably need to make specific adjustments anyway.

…g with this syntax

E.g.: `xmlns:xlink="http://www.w3.org/1999/xlink"`

This commit prevents this kinda error by enforcing the use of a
whitespace character before the dynamic attribute
Other syntaxes where clashing and they just didn't work properly
docs: update docs on change of default behavior
@vhoyer
Copy link
Author

vhoyer commented Feb 20, 2021

So, I've mauled over this problem for a while now, and I just can't find a way to make this work without changing the default value of g:vue_pre_processors to detect_on_enter.

Do we agree on changing the default value for this option? And if so, can someone help me with the tests that are breaking? 😅 sorry for the trouble 🙁

@adriaanzon
Copy link
Collaborator

I think the tests are failing because not all languages are loaded now. Maybe it's best to add support for setting the option to 'all', so that it loads all of the supported syntax files. This can then be used in our Vader tests and by users who want the old behavior back.

@vhoyer
Copy link
Author

vhoyer commented Feb 27, 2021

technically using all already work, right? 'cause of the

function! s:should_register(language, start_pattern)
  " Check whether a syntax file for {language} exists
  if empty(globpath(&runtimepath, 'syntax/' . a:language . '.vim'))
    return 0
  endif

  if exists('g:vue_pre_processors')
    if type(g:vue_pre_processors) == v:t_list
      return index(g:vue_pre_processors, s:language.name) != -1
    elseif g:vue_pre_processors is# 'detect_on_enter'
      return search(a:start_pattern, 'n') != 0
    endif
  endif

  return 1
endfunction

Should we add an explicity elseif g:vue_pre_processors is# 'all' too? maybe in that case also change the fallback return to 0? (that way the all keyword check would make more sense).

maybe just a comment on the script "hey, all is a valid keyword"?

@vhoyer vhoyer force-pushed the master branch 2 times, most recently from 769ded6 to c168b9c Compare February 27, 2021 15:03
@vhoyer
Copy link
Author

vhoyer commented Mar 30, 2021

also, I think it would be better from the user perspective if we mantain the ability to pre-load some resources, so how about we adopt an api like this:

  let g:vue_pre_processors = ['scss', 'detect_on_enter']

Thoughts?

@vhoyer
Copy link
Author

vhoyer commented Oct 12, 2022

Now it also highlights script stuff inside the template for other script languages like typescript and coffescript

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.

js syntax highlighting in @events and :bindings on template
2 participants