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

Consolidate JS, TS, LiveScript, CoffeeScript on common foundation #2518

Merged
merged 2 commits into from Apr 29, 2020

Conversation

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented Apr 28, 2020

Just the beginning, but it's a start.

@joshgoebel joshgoebel requested review from allejo and egor-rogov Apr 28, 2020
@joshgoebel joshgoebel changed the title WIP: Consolidate JS and TS on common foundation Consolidate JS and TS on common foundation Apr 28, 2020
@joshgoebel joshgoebel changed the title Consolidate JS and TS on common foundation Consolidate JS, TS, LiveScript on common foundation Apr 28, 2020
@joshgoebel joshgoebel added this to the 10.1 milestone Apr 28, 2020
@joshgoebel joshgoebel force-pushed the joshgoebel:consolidate_js_and_ts branch from daf9365 to 87bd22b Apr 28, 2020
@joshgoebel joshgoebel changed the title Consolidate JS, TS, LiveScript on common foundation Consolidate JS, TS, LiveScript, CoffeeScript on common foundation Apr 28, 2020
Copy link
Collaborator

@egor-rogov egor-rogov left a comment

I like the idea.
I didn't check all the keywords though, hope you did it right.

src/lib/modes.js Outdated Show resolved Hide resolved
className: 'meta',
begin: /^#![^\n]+sh\s*$/,
const SHEBANG = hljs.SHEBANG({
binary: /(fish|bash|zsh|sh)/,

This comment has been minimized.

@egor-rogov

egor-rogov Apr 29, 2020
Collaborator

There are more... What's wrong with previous .+sh approach?

This comment has been minimized.

@joshgoebel

joshgoebel Apr 29, 2020
Author Member

Then the relevance would have to change.. not every binary ending in sh is a shell... If we went a high relevance then we should have an explicit list.

This comment has been minimized.

@egor-rogov

egor-rogov Apr 29, 2020
Collaborator

I'm not sure what are the most frequently used shells (except for bash and historically sh, of course). Probably your list is good enough, I just don't know.

This comment has been minimized.

@joshgoebel

joshgoebel Apr 29, 2020
Author Member

If you'd like to suggest a few more to add, we have the room. :-) I'd rather add them slowly than have false positives. And if your concern is just highlighting the shebang itself (vs treating it as a comment) we could add a basic SHEBANG rule also with 0 relevancy - to catch shell like files with an unknown binary...

This comment has been minimized.

@joshgoebel

joshgoebel Apr 29, 2020
Author Member

I didn't look at that as a "match any *sh" I read that as match the path then sh... but you're right that before it would have been more generic. I guess I just dislike that - at least when it comes to appointing 10 relevancy.

This comment has been minimized.

@egor-rogov

egor-rogov Apr 29, 2020
Collaborator

If you'd like to suggest a few more to add, we have the room. :-)

I've heard of, say, ksh, but I have no idea if someone use it these days. I have no must-have suggestions.

And if your concern is just highlighting the shebang itself (vs treating it as a comment) we could add a basic SHEBANG rule also with 0 relevancy - to catch shell like files with an unknown binary...

I think it's a good idea.

This comment has been minimized.

@joshgoebel

joshgoebel Apr 29, 2020
Author Member

I expanded the list:

"fish",
    "bash",
    "zsh",
    "sh",
    "csh",
    "ksh",
    "tcsh",
    "dash",
    "scsh",
@joshgoebel joshgoebel force-pushed the joshgoebel:consolidate_js_and_ts branch from 87bd22b to edaefb7 Apr 29, 2020
Copy link
Collaborator

@egor-rogov egor-rogov left a comment

Looks good for me now.

joshgoebel added 2 commits Apr 28, 2020
Builds a common keyword foundation for any grammar that is
building on top of JavaScript:

- LiveScript
- CoffeeScript
- TypeScript

Also uses this common foundation for JS itself.
@joshgoebel joshgoebel force-pushed the joshgoebel:consolidate_js_and_ts branch from f40d942 to a6c1843 Apr 29, 2020
@joshgoebel joshgoebel merged commit a23f19e into highlightjs:master Apr 29, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants