-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
use ember-showdown-shiki #902
Conversation
✅ Deploy Preview for ember-api-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
1953f71
to
8b2f688
Compare
6212aab
to
52c799f
Compare
{{#if this.showClipboardSuccessIcon}} | ||
{{svg-jar 'success' width='24px' height='24px'}} | ||
{{else}} | ||
<CopyButton @clipboardText={{concat 'import ' @item " from '" @package "';"}} @title='Copy to clipboard' @success={{this.showSuccess}}> |
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.
thought: interesting, wasn't aware we had this functionality in the API docs. Could be something to build into ember-showdown-shiki
😄
suggestion: configure the languages to load in // ...
'ember-showdown-shiki': {
theme: 'github-dark',
languages: [
'bash',
'css',
'http',
'javascript',
'json',
'json5',
'ruby',
'scss',
'yaml',
'typescript',
'glimmer-js',
'glimmer-ts',
'handlebars',
],
}, |
Did some spot checks, and it looks all good to me 🙌 |
what is the alternative? does it load all of them? |
That's the default behaviour, yes. |
in that case I wouldn't consider this a blocker, we can fix it as an optimisation later. I wouldn't be comfortable limiting the languages on the api-docs unless we had some sort of lint checking that the input Markdowns are only using the right languages, does that make sense? |
The language grammars are loaded eagerly, so loading all of them does have a performance impact. I think it should be safe taking over the ones we set for the other guide apps. And if a language is missing, it would just be not highlighted (but not crash). The alternative is looking into making loading the grammars lazy in |
So a few things here: I'm not exactly sure it doesn't crash if the language doesn't exist. I had to do a process in ember-learn/ember-jsonapi-docs#137 that stripped the language As for dynamic loading of languages, I'm 100% against this. We need it to be predictable and I don't want to open up an issue of executing random JS modules from a remote location in a fastboot environment. I would rather the language be missing (and not crash) than loading it dynamically 👍 |
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.
LETS GOOOOOO 🚢
This makes use of the changes in ember-learn/ember-jsonapi-docs#137 and ember-learn/ember-api-docs-data#25 to make sure that we're using markdown/showdown for our descriptions everywhere now.
This has allowed us to use https://github.com/IgnaceMaes/ember-showdown-shiki which (finally) gives us the same functionality for syntax highlighting across guides and API docs 🎉