-
-
Notifications
You must be signed in to change notification settings - Fork 215
feat: HTML lexer supports vue bind attributes #1094
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
base: master
Are you sure you want to change the base?
Conversation
8b7b067
to
b616178
Compare
@PBK-B Thank you for the PR and kind words. Would you be open to create a separate vue lexer that extends the html-lexer? I'm a bit reluctant to add framework specific code to the html-lexer. Also, I'm confused as to why we need to define both lexers in the config file. Is that necessary? |
@karellm I seem to have seen that vue-lexer was deleted just now (although I can understand that it is not dependent on vue-template-compiler) secondly, configure both javascript-lexer and html-lexer for vue because the vue template syntax code file contains both javascript code and html-like code syntax. <template>
<button :aria-label="t('b_a_l', 'button aria label')">
button label
</button>
<button v-bind:aria-label="$t('button v-bind aria label')">
button label form v-bind
</button>
<p>{{t("tips", "this is an update prompt")}}</p>
<p>{{msg}}</p>
<p>{{elText}}</p>
</template>
<script lang="ts" setup>
import { computed, ref } from "vue"
import { t } from '@/i18n'
const msg = t("msg_text", "check for version updates")
const elText = computed(() => {
return getVerisonUpdate() ? t('new_version', "new version") : t("not_new_version", "no updates available")
})
</script> |
In addition, I am worried that adding back the vue-lexer that was originally removed and implementing inconsistent functions will cause confusion when people upgrade the version. |
@karellm I moved the code out separately and created a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @PBK-B
import JavascriptLexer from './javascript-lexer.js' | ||
import * as cheerio from 'cheerio' | ||
|
||
export default class VueTemplateLexer extends HTMLLexer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Why are you extending the HTMLLexer and then compose the jsLexer as well. For better maintainability, shouldn't it be extending from the BaseLexer and compose both the JSLexer and the HTMLLexer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geertklop Yes, by expanding BaseLexer, you can theoretically get better maintainability, but you will eventually find that it is actually a re-realization of HTMLLexer and JSLexer (excellent template syntax 🐶)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think that with the maintainability, I would expect to extend from BaseLexer. However, curious what regular maintainers like @karellm think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geertklop If you want, you can complete the follow-up work based on my branch
src/lexers/vue-template-lexer.js
Outdated
(item) => item.name.startsWith(':') || item.name.startsWith('v-bind:') | ||
) | ||
if (attributes.length > 0) { | ||
// there are calculation attributes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain a bit more what the 'calculation' means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geertklop I'm very sorry, the misunderstanding caused by my colloquial translation should actually be calculation attributes = Computed Properties
6cf74eb
to
d7396a5
Compare
d7396a5
to
49cd876
Compare
@karellm The master branch has been rebased. If there is anything I can do to promote its merger, I am very willing to work for it (so much so that I have to distribute a package by myself in the project at present). Thank you for your work. |
@karellm Hey, we need this 🗡️ |
Why am I submitting this PR
in Vue, there are v-bind-related attributes that can write simple syntax logic directly on the template syntax. This kind of code syntax exists in many traditional vue projects. I believe people will like that
t('translate')
written directly in v-bind can be recognized by i18next-parser.vue sample code:
the solution I tried is to directly transform the existing html-lexer by adding
vueBindAttr: Boolean
option to support the acquisition of:attribute="t('translate')"
andv-bind:attribute="t('translate')"
syntax sugar in the html tag, and then parse the functions in it through javascript-lexer, and finally get the desired results. This scheme does not introduce additional dependencies but will make html-lexer dependent on javascript-lexer when the function is enabled.usage example:
finally: @karellm TBR; At the same time, thank you for providing such a cool tool to save us a lot of time. I wish you a good day. If there is anything that can help promote this PR merger, welcome to comment.
Does it fix an existing ticket?
#617
Checklist
yarn test
(see details here)