highlightjs / highlightjs-extempore

Highlight.js Extempore language repository

BSD 3-Clause "New" or "Revised" License

2 stars 2 forks source link

Please update your README and bundle a `dist` build. #5

Closed joshgoebel closed 2 years ago

joshgoebel commented 3 years ago

Users finding your repo from the SUPPORTED_LANGUAGES list will expect to find up-to-date instructions on how to add your 3rd party grammar to their project. Your current instructions are very old and incorrect/outdated. There are now clear standards for how 3rd party grammar repos should be structured. Please see the examples below.

On the client-side ship a dist build (which our build script can help you generate). See https://github.com/highlightjs/highlight.js/blob/master/extra/3RD_PARTY_QUICK_START.md
On the server-side ship a npm module (that someone can use to extend our library).

Please see:

https://github.com/highlightjs/highlightjs-robots-txt (a example README)
https://github.com/highlightjs/highlight.js/issues/3008

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.

benswift commented 3 years ago

Hey @joshgoebel thanks for the heads up. @cyblue9 and I will get on it asap.

cyblue9 commented 3 years ago

Hi @joshgoebel

I tried to update. Updated branch is below.

https://github.com/highlightjs/highlightjs-extempore/tree/feature/update_latest_spec

Please review. If it is nothing problem, I merge this branch to main branch.

joshgoebel commented 3 years ago

I'd recommend changing:

<script type="text/javascript" src="/path/to/highlightjs-extempore/dist/extempore.min.js"></script>

<script type="text/javascript" src="/path/to/extempore.min.js"></script>

Someone using the file directly is likely to just by copying it or moving it (alone) into a better location, using it with the entire git checkout would be kind of strange - though I suppose you could do it. I'll leave changing it up to you, but I think I'd try to encourage someone to use just the single file vs deploying the whole repository to their project.

joshgoebel commented 3 years ago

N/m, now I see my own sample repo does this - though it's still bugging me - but I can't believe I haven't noticed till now. :) I'm going to give it more thought. :) Your call.

benswift commented 3 years ago

Hey @joshgoebel I don't think I have a strong opinion on this, although I do think that just copying in the minified file is probably easier & nicer than having to copy the whole directory. So unless there's some reason that you're keeping the "copy whole directory" instructions, we'll probably just do it the way you (initially) suggested. What do you reckon?

joshgoebel commented 3 years ago

So unless there's some reason that you're keeping the "copy whole directory" instructions

Well there are no instructions like that. :) It's just implied. :-) I'm going to change it now.

joshgoebel commented 3 years ago

https://github.com/highlightjs/highlightjs-robots-txt/commit/2c9d6e0707ea9511d825e57f18235170f84287f7

cyblue9 commented 3 years ago

Hi @joshgoebel

I changed README.md by referring to https://github.com/highlightjs/highlightjs-robots-txt/commit/2c9d6e0707ea9511d825e57f18235170f84287f7. Is it OK?

https://github.com/highlightjs/highlightjs-extempore/tree/feature/update_latest_spec

benswift commented 3 years ago

Btw I'll probably remove that "see how Ben does it on his blog" comment in the readme once this change lands, because this new way is better. But let's just get this thing figured out first.

joshgoebel commented 3 years ago

Looks good at a glance!

cyblue9 commented 3 years ago

@joshgoebel @benswift

Thank you for checking! I've fixed Ben's point and merged it into the main branch.