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

fix(build): line numbers mode when language specifier has symbol #1353

Merged
merged 2 commits into from
Sep 19, 2022

Conversation

sun0day
Copy link
Contributor

@sun0day sun0day commented Sep 17, 2022

fix #1352

Code block's language- class should compatible with special char, such as '-', '+', '#'...

After PR:

"language-" => "language- line-numbers-mode"
"language-js" => "language-js line-numbers-mode"
"language-c#" => "language-c# line-numbers-mode"
"language-c++" => "language-c++ line-numbers-mode"
"language-Objective-C" => "language-Objective-C line-numbers-mode"

Copy link
Member

@brc-dd brc-dd left a comment

Choose a reason for hiding this comment

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

This won't work with string like this:

<div class="language-">..."...</div>

Probably something like /"(language-[^\s"]*?)"/ should work

@sun0day
Copy link
Contributor Author

sun0day commented Sep 17, 2022

use /"(language-.*?)"/ ?

/"(language-[^\s"]*?)"/ dont work with <div class="language- xxxx">..."...</div>

@brc-dd
Copy link
Member

brc-dd commented Sep 17, 2022

There won't be any <div class="language- xxxx">..."...</div> case if that div is generated by our code highlighting plugin. However, I suppose we can simplify that regex to: /"(language-\S*?)"/

@sun0day
Copy link
Contributor Author

sun0day commented Sep 17, 2022

There won't be any <div class="language- xxxx">..."...</div> case if that div is generated by our code highlighting plugin.

From the current code, yes, this case won't happen.

But what I concern is that from the lineNumberPlugin API aspect, it is better to be compatible with more cases.

So which one do you prefer @brc-dd ? If you insist, I'll change the regex to /"(language-\S*?)"/

@brc-dd
Copy link
Member

brc-dd commented Sep 18, 2022

Change it to either. I'd still recommend \S one, we should keep regexes stricter because they tend to mess back 😅. However, in this case both should work fine IG.

@sun0day
Copy link
Contributor Author

sun0day commented Sep 19, 2022

Change it to either. I'd still recommend \S one, we should keep regexes stricter because they tend to mess back 😅. However, in this case both should work fine IG.

Alright. Let's change it to /"(language-\S*?)"/ for now.

Done. Merge it at your convenience. @brc-dd

@brc-dd brc-dd changed the title fix: code language compatible with special char (fix #1352) fix(build): line numbers mode when language specifier has symbol Sep 19, 2022
@brc-dd brc-dd merged commit 9c04a10 into vuejs:main Sep 19, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

language-c# missing 'line-numbers-mode' class
2 participants