highlightjs / highlight.js

JavaScript syntax highlighter with language auto-detection and zero dependencies.
https://highlightjs.org/
BSD 3-Clause "New" or "Revised" License
23.31k stars 3.52k forks source link

Fix interface LanguageDetail > keywords #3961

Closed patrick-kw-chiu closed 5 months ago

patrick-kw-chiu commented 5 months ago

Resolves #3936 (though the issue is closed as completed)

Changes

Added RegExp to keywords, to reflect the $pattern field in python and dart.

https://github.com/highlightjs/highlight.js/blob/6a52185d9b855130b5acaccef143a7bd602e7885/src/languages/python.js#L148-L154

Checklist

joshgoebel commented 5 months ago

Hmmm, this type is a bit broad... doesn't TypeScript have a way to merge types or something so we could have the Record but also merge in the very specific { $pattern: RegExp } on top of that? That would be a lot more accurate.

It's been a while but I seem to remember something like that in TS.

patrick-kw-chiu commented 5 months ago

Make sense.. I guess we can't do something like: "All keys have a string type, but only a specific key ($pattern) have string | RegExp". Source and TypeScript playground 1

The best we can do is to define { $pattern: RegExp }. Then extend it by specifying an exclusive list of keys - "keyword" and "built_in" to have the string | string[] type. TypeScript playground 2. (P.S. it doesn't seem to be able to define in 1 shot. It will yield the A mapped type may not declare properties or methods error. TypeScript playground 3)

Does it sound more accurate to you? If so, you can let me know the exclusive list of keys in the keywords field. I can make the changes accordingly.

joshgoebel commented 5 months ago

The best we can do

Don't love it... really people should be free to define their own keyword scopes (in 3rd party grammars) and then provide matching CSS for that... so I think we'll merge this as-is... thanks for the additional research though.

Perhaps this is a good reason to consider lifting that key out of keywords into a top level key such as keywords_pattern: or some such... that would at least be simpler WRT to TypeScript I think.

Thoughts?

github-actions[bot] commented 5 months ago

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

3 files changed

Total change +5 B

View Changes | file | base | pr | diff | | --- | --- | --- | --- | | es/core.min.js | 8.17 KB | 8.18 KB | +2 B | | es/highlight.min.js | 8.17 KB | 8.18 KB | +2 B | | highlight.min.js | 8.21 KB | 8.21 KB | +1 B |
patrick-kw-chiu commented 5 months ago

Ah, I see what you mean. I orginally thought the fields in keyword are exclusive 🙈

If we lift keywords_patrern out of keywords, it seems to give me an impression that, the behaviour of keywords_pattern is different than keywords. Not sure is it the case? Other than that, I don't have much comments 👍

joshgoebel commented 5 months ago

The pattern is the regex used to identify possible keywords before they are matched against the literal list... I thought it was kind of "cool" to prefix it with $ and make it part of the keywords hash itself - to distinguish it as a config element... I