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

(TypeScript) Camel-case type parameters: only first component is highlighted #3403

Closed
cakoose opened this issue Nov 20, 2021 · 8 comments · Fixed by #3411
Closed

(TypeScript) Camel-case type parameters: only first component is highlighted #3403

cakoose opened this issue Nov 20, 2021 · 8 comments · Fixed by #3411
Labels
bug help welcome Could use help from community language

Comments

@cakoose
Copy link

cakoose commented Nov 20, 2021

Describe the issue

In the type parameter "OutT", the "Out" is highlighted differently from the "T".

Which language seems to have the issue?

typescript

Are you using highlight or highlightAuto?

Not sure. I noticed this issue on StackOverflow and reproduced with your JSFiddle template.

Sample Code to Reproduce

https://jsfiddle.net/650rLwyd/

Screenshot:

Screen Shot 2021-11-20 at 15 30 25

Expected behavior

GitHub's highlighting in markdown code fences looks better:

type ParseFunc<OutT, InT> = (val: InT) => OutT;

declare const parseString: ParseFunc<string, unknown>;

declare const makeParseObject: <FPs extends {[s: string]: ParseFunc<any, any>}> (fieldParsers: FPs)
    => ParseFunc<ParseObjectResult<FPs>, unknown>;
type ParseObjectResult<FPs extends {[s: string]: ParseFunc<any, unknown>}> =
    {[F in keyof FPs]: ParseResult<FPs[F]>}
type ParseResult<P> = P extends ParseFunc<infer OutT, unknown> ? OutT : never;

Screenshot, just in case it changes:

Screen Shot 2021-11-20 at 15 27 32

Additional context

@cakoose cakoose added bug help welcome Could use help from community language labels Nov 20, 2021
@joshgoebel
Copy link
Member

Yeah I'd say that's a bug with our camelcase class name detection.

And some typescript keywords we should probably add.

@joshgoebel
Copy link
Member

Are there other TS keywords we need to add?

@cakoose
Copy link
Author

cakoose commented Nov 29, 2021

Kind of a tangent, but some keywords are context-sensitive. For example, type is commonly used as a normal identifier:

const type = 'abc';
type X = string;

This is also true for other keywords, e.g. infer, readonly, keyof.

Not sure if highlight.js can be enhanced to support this kind of thing, but it appears GitHub's highlighting does.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 29, 2021

Not sure if highlight.js can be enhanced to support this kind of thing,

Depends... the [kw] = case (assignment, not KW) could be handled easily enough with a negative look-ahead. Someone would have to lay out cases for each of the keywords in question and we've honestly have to look at them one by one.

Short of doing that we can either highlight none of them or highlight them always regardless.

@joshgoebel
Copy link
Member

FYI: We do only turn these on for TS code, so they don't false highlight JS code - only TS code.

@cakoose
Copy link
Author

cakoose commented Nov 29, 2021

I have no idea how much work it is but just in case it's helpful, here's the TS source code that defines the list of "reserved words" vs "contextual keywords":

https://github.com/Microsoft/TypeScript/blob/fad889283e710ee947e8412e173d2c050107a3c1/src/compiler/types.ts#L86-L148

@joshgoebel
Copy link
Member

This isn't super helpful though, what might be more useful is a literal list (not a list of named symbols). I don't even see keyof and infer listed there at all...

Contextual keywords are always something we're going to struggle with... if there was an absolute list we should probably just not include any of them (rather than suffer false positives).

I may just back out my keyword changes and then close this with the PR that addresses only the camel-case, as that was the bulk of this issue. contextual keywords is a whole other thing.

@cakoose
Copy link
Author

cakoose commented Dec 3, 2021

Sorry, I got that link from another GH issue and it pointed to an older version of the file. Here's the latest version link (includes keyof and infer, though those are both "contextual").

And here's the list that maps the literal to the code identifier: link

I agree that it's a good idea to limit this PR to address just the camel-case thing. Maybe it's worth opening a new issue for this keyword discussion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help welcome Could use help from community language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants