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

Improved grammar and syntax #1093

Merged
merged 16 commits into from
Feb 20, 2019
Merged

Improved grammar and syntax #1093

merged 16 commits into from
Feb 20, 2019

Conversation

p-kuen
Copy link
Contributor

@p-kuen p-kuen commented Feb 14, 2019

to make this possible I had to make two functions async
  (for dynamic vscode import)

alternative: put the generateGrammarCommandHandler into a seperate file
syntaxes/vue.yaml Outdated Show resolved Hide resolved
client/grammar.ts Outdated Show resolved Hide resolved
client/grammar.ts Show resolved Hide resolved
syntaxes/vue.tmLanguage.json Outdated Show resolved Hide resolved
@octref
Copy link
Member

octref commented Feb 15, 2019

This case still seems broken:

image

<template lang="pug">
section
  router-link.header.item(to=`/`)
    img(
      src=`/images/navbar-icon.svg`
      :alt=`title`
      )

  .right.menu
    navbar-menu
</template>

@p-kuen
Copy link
Contributor Author

p-kuen commented Feb 15, 2019

Yeah, that's right. I tried many things but got no good solution - so this case would be a bit trickier.
I would prefer to use a space after the title keyword or put it into multiple lines if using the ` literal.

@octref
Copy link
Member

octref commented Feb 15, 2019

If supporting backtick would bring in too much complexity I suggest dropping it, documenting it in the https://vuejs.github.io/vetur/highlighting.html page and moving on.

Copy link
Member

@octref octref left a comment

Choose a reason for hiding this comment

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

Just small nitpicks. Please resolve the path and leave using \/ for later. This looks great. Thanks a lot for adding the test infrastructure!

test/grammar.test.ts Outdated Show resolved Hide resolved
p-kuen and others added 4 commits February 20, 2019 09:15
Co-Authored-By: Patcher56 <patrick.kuen11@gmail.com>
- using path.resolve for paths now
- showing better error messages if assertion fails
@octref
Copy link
Member

octref commented Feb 20, 2019

👍 Thanks a lot for the quick response!

@octref octref merged commit b298d75 into vuejs:master Feb 20, 2019
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