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

Make ScriptElementKind and HighlightSpanKind string enums #15966

Merged
6 commits merged into from
May 23, 2017
Merged

Make ScriptElementKind and HighlightSpanKind string enums #15966

6 commits merged into from
May 23, 2017

Conversation

ghost
Copy link

@ghost ghost commented May 20, 2017

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.


/**
* <JsxTagName attribute1 attribute2={0} />
*/
export const jsxAttribute = "JSX attribute";
jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {
Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

and ClassificationTypeNames

Copy link
Contributor

Choose a reason for hiding this comment

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

also server.CommandTypes

Copy link
Contributor

Choose a reason for hiding this comment

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

and HighlightSpanKind


/**
* <JsxTagName attribute1 attribute2={0} />
*/
export const jsxAttribute = "JSX attribute";
jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {
Copy link
Contributor

Choose a reason for hiding this comment

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

and ClassificationTypeNames


/**
* <JsxTagName attribute1 attribute2={0} />
*/
export const jsxAttribute = "JSX attribute";
jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {
Copy link
Contributor

Choose a reason for hiding this comment

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

also server.CommandTypes


/**
* <JsxTagName attribute1 attribute2={0} />
*/
export const jsxAttribute = "JSX attribute";
jsxAttribute = "JSX attribute",
}

export namespace ScriptElementKindModifier {
Copy link
Contributor

Choose a reason for hiding this comment

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

and HighlightSpanKind

@mhegazy
Copy link
Contributor

mhegazy commented May 22, 2017

@mhegazy mhegazy added Breaking Change Would introduce errors in existing code API Relates to the public API for TypeScript labels May 22, 2017
@mhegazy mhegazy added this to the TypeScript 2.4 milestone May 22, 2017
@ghost
Copy link
Author

ghost commented May 22, 2017

@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).

@@ -2,103 +2,103 @@
* Declaration module describing the TypeScript Server protocol
*/
namespace ts.server.protocol {
export namespace CommandTypes {
export type Brace = "brace";
export enum CommandTypes {
Copy link
Contributor

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.

Copy link
Author

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 {
Copy link
Contributor

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
Copy link
Contributor

mhegazy commented May 22, 2017

@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.

@ghost ghost merged commit 73ee2fe into master May 23, 2017
@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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Relates to the public API for TypeScript Breaking Change Would introduce errors in existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants