- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.1k
Make ScriptElementKind and HighlightSpanKind string enums #15966
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
Conversation
        
          
                src/services/types.ts
              
                Outdated
          
        
      | jsxAttribute = "JSX attribute", | ||
| } | ||
|  | ||
| export namespace ScriptElementKindModifier { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should also change ScriptElementKindModifier to be an enum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and ClassificationTypeNames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also server.CommandTypes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and HighlightSpanKind
        
          
                src/services/types.ts
              
                Outdated
          
        
      | jsxAttribute = "JSX attribute", | ||
| } | ||
|  | ||
| export namespace ScriptElementKindModifier { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and ClassificationTypeNames
        
          
                src/services/types.ts
              
                Outdated
          
        
      | jsxAttribute = "JSX attribute", | ||
| } | ||
|  | ||
| export namespace ScriptElementKindModifier { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also server.CommandTypes
        
          
                src/services/types.ts
              
                Outdated
          
        
      | jsxAttribute = "JSX attribute", | ||
| } | ||
|  | ||
| export namespace ScriptElementKindModifier { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and HighlightSpanKind
| Also please add a note for this in https://github.com/Microsoft/TypeScript/wiki/API-Breaking-Changes#typescript-24 | 
| @mhegazy Is there any reason to avoid a non- | 
        
          
                src/server/protocol.ts
              
                Outdated
          
        
      | namespace ts.server.protocol { | ||
| export namespace CommandTypes { | ||
| export type Brace = "brace"; | ||
| export enum CommandTypes { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be const enum, since it does not exist at time of building the client against the protocol file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to iterate over it though -- I guess I could define a CommandTypes[] elsewhere but we'd have to remember to update both places every time a new command is added.
| export namespace NewLineKind { | ||
| export type Crlf = "Crlf"; | ||
| export type Lf = "Lf"; | ||
| export const enum NewLineKind { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing the initialization here.
| 
 yes. protocol.ts compiles down to  so either we make them const in  | 
Note: is technically a change to our public API as many
strings are nowScriptElementKinds. And anyone using the API must now have ts@next or they will get compile errors on the string enums -- but presumably people compiling against the ts@next API are compiling using ts@next too.Required running
gulp LKGto get #15486 in.