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

Add new aliases #2390

Merged
merged 3 commits into from May 17, 2020
Merged

Add new aliases #2390

merged 3 commits into from May 17, 2020

Conversation

@valtlai
Copy link
Contributor

@valtlai valtlai commented May 17, 2020

Adds aliases:

valtlai added 2 commits May 17, 2020
* atom and rss for markup
* webmanifest for json
Reorder aliases in components.json to match those in prism-markup.js
@RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented May 17, 2020

Thank you for making this PR @valtlai!

Could you please clarify what "Atom" is? I only know this Atom and this atom.

It seems that the name atom collides with something, since it doesn’t get added to plugins/show-language/prism-show-language.js (or its minified version) when running gulp. I couldn’t find the reason for this.

Most language ids are just the name of the language in all-lowercase, so Show Language will try to guess the title by capitalizing the id. If that guess is correct, then the id-title pair won't be stored in prism-show-language.js.
If I remember correctly, we guess about 30% correctly, so this decreases the file size quite a bit.

Also, don't worry. If any aliases/ids collide, the tests won't pass.

@valtlai
Copy link
Contributor Author

@valtlai valtlai commented May 17, 2020

Hi, @RunDevelopment!

Could you please clarify what "Atom" is? I only know this Atom and this atom.

It’s Atom feed, like RSS but less known.

Most language ids are just the name of the language in all-lowercase, so Show Language will try to guess the title by capitalizing the id. If that guess is correct, then the id-title pair won't be stored in prism-show-language.js.
If I remember correctly, we guess about 30% correctly, so this decreases the file size quite a bit.

Oh, that’s clever!

@RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented May 17, 2020

Thanks for the clarification.

One last thing before we merge this: Could you please revert the reordering? The ordering in prism-markup.js is a pure implementation detail and has nothing to do with the order in components.json. Also, I like the older order because it's roughly sorted from most-used to least-used.
I think it's ok if we just append new aliases. (If we created some rule for their ordering then we'd also have to write a test that enforces that rule.)

@RunDevelopment RunDevelopment merged commit 9782cfe into PrismJS:master May 17, 2020
1 check passed
@RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented May 17, 2020

Thank you for contributing @valtlai!

@valtlai valtlai deleted the aliases branch May 17, 2020
quentinvernot added a commit to TankerHQ/prismjs that referenced this issue Sep 11, 2020
This adds the `rss` and `atom` alias for Markup and the `webmanifest` alias for JSON.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants