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

Markup: Added support type="text/plain" scripts #2442

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

Conversation

RunDevelopment
Copy link
Member

This adds support for properly unhighlighted <script type="text/plain"> regions:

image


The main reason I implemented this isn't text/plain. It's Vue (#1665).

Vue supports embedded script and style tags with languages other than Javascript and CSS. It differentiates the languages based on a lang attribute.

The problem here is that the lang attribute can be anywhere in the list of attributes. So I changed the addInlined function to support arbitrary attributes as additional conditions. Example:

// this will only highlight script tags with a `lang="ts"` or `lang='ts'` or `lang=ts` attribute as TS
addInlined('script', 'typescript', [{ name: /lang/, value: /ts/ }]);

It supports multiple attributes and will generate a regex that only matches tags that fit all criteria.


After this and #1991 are through, I can implement Vue.

@mAAdhaTTah
Copy link
Member

The main reason I implemented this isn't text/plain. It's Vue (#1665).

I wonder if maybe this code should be moved into a "Vue" language? Bot hasn't spoken yet, but this seems like a lot of code to add to the core language for a non-default use case.

@github-actions
Copy link

github-actions bot commented Dec 22, 2020

JS File Size Changes (gzipped)

A total of 4 files have changed, with a combined diff of +623 B (+12.0%).

file master pull size diff % diff
components/prism-markup-attributes.min.js 0 Bytes 433 B +433 B +100.0%
components/prism-markup.min.js 897 B 1.07 KB +172 B +19.2%
plugins/autoloader/prism-autoloader.min.js 2.15 KB 2.16 KB +9 B +0.4%
plugins/show-language/prism-show-language.min.js 2.15 KB 2.16 KB +9 B +0.4%

Generated by 🚫 dangerJS against 5afc2f9

@RunDevelopment
Copy link
Member Author

Bot has spoken and it doesn't look good.

Maybe we could make addInline its own "language"? But JS and CSS need it, so I don't know if it'll help.

@mAAdhaTTah
Copy link
Member

Maybe we could make addInline its own "language"? But JS and CSS need it, so I don't know if it'll help.

Yeah, the issue isn't the function as a whole; it's the extra logic required to support this use case. I don't know if I have a solution off the top tho.

@RunDevelopment
Copy link
Member Author

The new function is basically a generalization of the old one. I could factor out the code that generates the attribute loop (= the new generalization) and replace it with a function. That could work. I'll look for a solution tomorrow.

@RunDevelopment
Copy link
Member Author

Hmmm. Markup still gains about 200B but I don't think that I can scrap off some more.

components.json Outdated
Comment on lines 714 to 718
"markup-attributes": {
"title": "Markup attributes",
"require": "markup",
"owner": "RunDevelopment"
},
Copy link
Member

Choose a reason for hiding this comment

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

If what we're doing now is designed primarily for Vue, would it be worth just making this a full-fledged Vue language for Vue SFCs? If we need this more generally for multiple languages, we can extract it then, but markup-attributes isn't terribly clear as to what the languages actually does.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that the modification it makes to markup justifies it being a "language". It would be kinda weird if Vue added support for plain text scripts in HTML. Honestly, the whole thing should be split even further: 1 "language" with just the attributes function and 1 language that adds support for plain text scripts.

Also, it's not like the whole thing is useful for just Vue and HTML. ASP.net could also profit from it but I'll need to make some more adjustments for this.

but markup-attributes isn't terribly clear as to what the languages actually does.

True. I can't think of a better name thou...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants