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

Please add a full README (with examples) and dist CDN build #5

Open
joshgoebel opened this issue Mar 1, 2021 · 7 comments
Open

Please add a full README (with examples) and dist CDN build #5

joshgoebel opened this issue Mar 1, 2021 · 7 comments

Comments

@joshgoebel
Copy link
Member

Users finding your repo from the SUPPORTED_LANGUAGES list will expect to find instructions on how to add your 3rd party grammar to their project.

Please see:

#3008 is specifically related to upgrading from v10 to v11 but it still applies to the proper way to structure your repo to make it easiest to use for the most people.

@mossheim
Copy link
Contributor

mossheim commented Mar 1, 2021

hey @joshgoebel -- sorry to say that i haven't had the best experience working with devs from highlightjs because there's been a lack of clarity about how or if i can get help integrating. in particular, this comment was super disappointing. i'm also not sure whether #1 still applies to this repo. so, due to these reasons and a general lack of time to put into OSS work, i will probably not be acting on this issue or maintaining this repo going forward.

@joshgoebel
Copy link
Member Author

joshgoebel commented Mar 2, 2021

My apologies for any lack of clarity or disappointment. Things were indeed confusing in 2018 when you originally opened your PR... Sorry if you felt like you were given the run around. I think at that point we'd stopped accepting grammar PRs but it wasn't made explicit... but since then we now have a clear "path forward" and plenty of clarity for those providing 3rd party grammars. It is not the answer every single person wants to hear, but we have been very consistent since we established the 3rd party grammar spec last year in the early v10 days.

I'm not sure if I've explained before (I have in many other threads), but getting a language "upstreamed" to a 3rd party service isn't magic - even if we were still accepting grammar PRs to core. Often services choose a specific subset on purpose (for speed, size, security, and maintenance reasons) so getting a 3rd party services to support even a built-in grammar is often an uphill battle (requiring groveling, etc) - entirely separate from any policy we have about merging grammar PRs.

For example this has led to a lot of confusion/discussion on StackOverflow... since they choose to only support a small subset of the +180 languages we support in core. People find out they now use us for highlighting and then assume that means they would highlight all languages we support. Nope, far from it.

a lack of clarity about how or if i can get help integrating.

This issue is actually specifically about that - making it easier for end users to find, use, and integrate your grammar into their projects (if they choose to do so). Making a grammar available on npm and providing a CDN dist module increases the chances that end users will choose to integrate it into their projects. But yes, it is always the end users choice regarding which grammars they choose to build into their project.

Core does include a "common" set (our default build), but we don't force it on anyone.

general lack of time to put into OSS work, i will probably not be acting on this issue or maintaining this repo going forward.

Understood. You may wish to clone this repo to your personal GitHub (if you haven't already) for the long-term. At some point in the future (if it goes unmaintained) we may archive it, or eventually remove it.

@mossheim
Copy link
Contributor

mossheim commented Mar 2, 2021

thanks for the apology. :)

The main project that I want to support this module is Discourse. Judging from these two comments from 2019: https://meta.discourse.org/t/how-to-get-solidity-syntax-highlighting/117774/6, highlightjs/highlight.js#2051 (comment), it appears to me Discourse isn't limiting the languages available on purpose, but rather because they just haven't figured out how to integrate these additional modules. I haven't seen any communication between highlightjs and Discourse maintainers since then, so maybe it's worth reaching out to them to see if this new dist module effort makes sense to them and whether they would adopt it or provide a way for self-hosted instances to do so? I can also make a post on Discourse Meta to ask about this if you want. It would probably help a lot of people.

@joshgoebel
Copy link
Member Author

joshgoebel commented Mar 2, 2021

Ok, I skimmed all this again just now. Are you familiar with theme modules? It looks trivially easy from what this other person has posted. Here is there theme thingy they provided:

https://github.com/pmusaraj/discourse-highlight.js-solidity/blob/master/common/header.html

<script type="text/discourse-plugin" version="0.8.27">
api.registerHighlightJSLanguage("solidity", function(e){ // ...
</script>

All you need to do is build a CDN distributable (one of the tasks this issue was asking you to do)... then edit it slightly. Lets take the robots-txt example:

hljs.registerLanguage("robots-txt",(function(e){var s=e.COMMENT("#","$"); // ...

Simply replacing the hl's API with Discourse's own API:

api.registerHighlightJSLanguage("robots-txt",(function(e){var s=e.COMMENT("#","$"); // ...

I'm really not sure why their support couldn't have helped you with this, or perhaps api.registerHighlightJSLanguage is a new feature that was added more recently?

@joshgoebel
Copy link
Member Author

It's probably possible to also make a "more official" theme plugin [or whatever they call them] (perhaps even host it inside this repository right besides the grammar)... to make it easier for others to use your plugin on Discourse.

At the very least you could add another section to your README once you got it working.

@mossheim
Copy link
Contributor

mossheim commented Mar 2, 2021

Thanks! I'm going to reach out to Discourse and see what they say.

@joshgoebel
Copy link
Member Author

Any luck?

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

No branches or pull requests

2 participants