Make ScriptElementKind and HighlightSpanKind string enums · Pull Request #15966 · microsoft/TypeScript

@ghost

Note: is technically a change to our public API as many strings are now ScriptElementKinds. 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 LKG to get #15486 in.

mhegazy

jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and ClassificationTypeNames

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also server.CommandTypes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and HighlightSpanKind

mhegazy

jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and ClassificationTypeNames

jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also server.CommandTypes

jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and HighlightSpanKind

@mhegazy

Andy Hanson added 2 commits

May 22, 2017 10:40

@ghost

@mhegazy Is there any reason to avoid a non-const enum in protocol.ts? We iterate over for (const name in CommandNames) in a unit test (and now CommandNames = CommandTypes).

mhegazy

namespace ts.server.protocol {
export namespace CommandTypes {
export type Brace = "brace";
export enum CommandTypes {

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.

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.

mhegazy

export namespace NewLineKind {
export type Crlf = "Crlf";
export type Lf = "Lf";
export const enum NewLineKind {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing the initialization here.

@mhegazy

@mhegazy Is there any reason to avoid a non-const enum in protocol.ts? We iterate over for (const name in CommandNames) in a unit test (and now CommandNames = CommandTypes).

yes. protocol.ts compiles down to protocol.d.ts which is then used by the clients to compile against.

so either we make them const in protocol.ts or do that while we put together protocol.d.ts in buildProtocol.js.

mhegazy

@ghost ghost deleted the kind branch

May 23, 2017 14:19

@microsoft microsoft locked and limited conversation to collaborators

Jun 14, 2018

This pull request was closed.