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.

Sign up for GitHub

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

When producing types declarations, also emit the static types of the interfaces for TypeScript declarations. #284

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

When producing types declarations, also emit the static types of the interfaces for TypeScript declarations. #284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6551cf3
When producing types declarations, also emit the static types of the …
ashgti Jan 5, 2023
e7259f7
Clean up the event handler in the adpater.
ashgti Jan 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions adapter/src/debugSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ export class CompletionItem implements DebugProtocol.CompletionItem {
}

export class StoppedEvent extends Event implements DebugProtocol.StoppedEvent {
event: 'stopped';

body: {
reason: string;
};
Expand All @@ -158,6 +160,8 @@ export class StoppedEvent extends Event implements DebugProtocol.StoppedEvent {
}

export class ContinuedEvent extends Event implements DebugProtocol.ContinuedEvent {
event: 'continued';

body: {
threadId: number;
};
Expand All @@ -175,12 +179,16 @@ export class ContinuedEvent extends Event implements DebugProtocol.ContinuedEven
}

export class InitializedEvent extends Event implements DebugProtocol.InitializedEvent {
event: 'initialized';

public constructor() {
super('initialized');
}
}

export class TerminatedEvent extends Event implements DebugProtocol.TerminatedEvent {
event: 'terminated';

public constructor(restart?: any) {
super('terminated');
if (typeof restart === 'boolean' || restart) {
Expand All @@ -193,6 +201,8 @@ export class TerminatedEvent extends Event implements DebugProtocol.TerminatedEv
}

export class ExitedEvent extends Event implements DebugProtocol.ExitedEvent {
event: 'exited';

body: {
exitCode: number
};
Expand All @@ -206,6 +216,8 @@ export class ExitedEvent extends Event implements DebugProtocol.ExitedEvent {
}

export class OutputEvent extends Event implements DebugProtocol.OutputEvent {
event: 'output';

body: {
category: string,
output: string,
Expand All @@ -225,6 +237,8 @@ export class OutputEvent extends Event implements DebugProtocol.OutputEvent {
}

export class ThreadEvent extends Event implements DebugProtocol.ThreadEvent {
event: 'thread';

body: {
reason: string,
threadId: number
Expand All @@ -240,6 +254,8 @@ export class ThreadEvent extends Event implements DebugProtocol.ThreadEvent {
}

export class BreakpointEvent extends Event implements DebugProtocol.BreakpointEvent {
event: 'breakpoint';

body: {
reason: string,
breakpoint: DebugProtocol.Breakpoint
Expand All @@ -255,6 +271,8 @@ export class BreakpointEvent extends Event implements DebugProtocol.BreakpointEv
}

export class ModuleEvent extends Event implements DebugProtocol.ModuleEvent {
event: 'module';

body: {
reason: 'new' | 'changed' | 'removed',
module: DebugProtocol.Module
Expand All @@ -270,6 +288,8 @@ export class ModuleEvent extends Event implements DebugProtocol.ModuleEvent {
}

export class LoadedSourceEvent extends Event implements DebugProtocol.LoadedSourceEvent {
event: 'loadedSource';

body: {
reason: 'new' | 'changed' | 'removed',
source: DebugProtocol.Source
Expand All @@ -285,6 +305,8 @@ export class LoadedSourceEvent extends Event implements DebugProtocol.LoadedSour
}

export class CapabilitiesEvent extends Event implements DebugProtocol.CapabilitiesEvent {
event: 'capabilities';

body: {
capabilities: DebugProtocol.Capabilities
};
Expand All @@ -298,6 +320,8 @@ export class CapabilitiesEvent extends Event implements DebugProtocol.Capabiliti
}

export class ProgressStartEvent extends Event implements DebugProtocol.ProgressStartEvent {
event: 'progressStart';

body: {
progressId: string,
title: string
Expand All @@ -316,6 +340,8 @@ export class ProgressStartEvent extends Event implements DebugProtocol.ProgressS
}

export class ProgressUpdateEvent extends Event implements DebugProtocol.ProgressUpdateEvent {
event: 'progressUpdate';

body: {
progressId: string
};
Expand All @@ -332,6 +358,8 @@ export class ProgressUpdateEvent extends Event implements DebugProtocol.Progress
}

export class ProgressEndEvent extends Event implements DebugProtocol.ProgressEndEvent {
event: 'progressEnd';

body: {
progressId: string
};
Expand All @@ -348,6 +376,8 @@ export class ProgressEndEvent extends Event implements DebugProtocol.ProgressEnd
}

export class InvalidatedEvent extends Event implements DebugProtocol.InvalidatedEvent {
event: 'invalidated';

body: {
areas?: DebugProtocol.InvalidatedAreas[];
threadId?: number;
Expand All @@ -371,6 +401,8 @@ export class InvalidatedEvent extends Event implements DebugProtocol.Invalidated
}

export class MemoryEvent extends Event implements DebugProtocol.MemoryEvent {
event: 'memory';

body: {
memoryReference: string;
offset: number;
Expand Down
2 changes: 2 additions & 0 deletions adapter/src/messages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export class Message implements DebugProtocol.ProtocolMessage {
}

export class Response extends Message implements DebugProtocol.Response {
type: 'response';
request_seq: number;
success: boolean;
command: string;
Expand All @@ -35,6 +36,7 @@ export class Response extends Message implements DebugProtocol.Response {
}

export class Event extends Message implements DebugProtocol.Event {
type: 'event';
event: string;

public constructor(event: string, body?: any) {
Expand Down
2 changes: 2 additions & 0 deletions adapter/src/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ export class ProtocolServer extends ee.EventEmitter implements VSCodeDebugAdapte
this._pendingRequests.delete(response.request_seq);
clb(response);
}
} else if (msg.type === 'event') {
this._emitEvent(<DebugProtocol.Event>msg)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about this, it seems like the intent is not to emit DAP events but just errors and the like.

Copy link
Member

Choose a reason for hiding this comment

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

The emitter is typed as emitting DebugProtocolMessage's, but that's just an empty interface 🤷 I think it'd make sense to emit them here

Copy link
Member

Choose a reason for hiding this comment

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

But it's weird that it's also emitting non-DAP errors from the same emitter. But also that events currently aren't going anywhere? Do you have some more context on how this works at all currently @ashgti?

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure if there was a way to get DAP events if you used the adapter programmatically, but when I added the types here I noticed that events weren't being handled.

I can revert this part, it just seemed like a missing piece of the adapter.

Copy link
Member

Choose a reason for hiding this comment

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

I think this method is only handling the data stream from the client, so this would only be hit if the client sends an event. I don't think that happens for any event types, or does it?

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't happen for any event types, removed.

}
}

Expand Down
13 changes: 5 additions & 8 deletions protocol/src/generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ function Module(moduleName: string, schema: IProtocol): string {
s += line(" *--------------------------------------------------------------------------------------------*/");
s += line();

//s += comment(schema.description);
s += comment({ description: 'Declaration module describing the VS Code debug protocol.\nAuto-generated from json schema. Do not edit manually.'});

s += line();

s += comment({ description: schema.description });
s += openBlock(`export declare module ${moduleName}`);

for (let typeName in schema.definitions) {
Expand Down Expand Up @@ -77,7 +78,7 @@ function Interface(interfaceName: string, definition: P.Definition, superType?:

let s = line();

s += comment({ description : desc });
s += comment({ description: desc });

let x = `interface ${interfaceName}`;
if (superType) {
Expand Down Expand Up @@ -247,11 +248,7 @@ function property(name: string, optional: boolean, prop: P.PropertyType): string
s += comment(prop);
const type = propertyType(prop);
const propertyDef = `${name}${optional ? '?' : ''}: ${type}`;
if (type[0] === '\'' && type[type.length-1] === '\'' && type.indexOf('|') < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, can you give an example of what changed here?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, it's basically all of the type/event/command parameters. It's not clear to me why we are emitting these commented out. And does add a constraint but, for example, if you implemented one of the event types and set event different, then that would be wrong and the event wouldn't even be handled correctly. So I feel fine about that change.

Copy link
Author

Choose a reason for hiding this comment

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

I think TypeScript will check that the event type (or type, etc.) matches the expected value if you where to try to use the wrong type.

s += line(`// ${propertyDef};`);
} else {
s += line(`${propertyDef};`);
}
s += line(`${propertyDef};`);
return s;
}

Expand Down