-
Notifications
You must be signed in to change notification settings - Fork 816
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
json schema for protocol #40
Conversation
Hi @gorkem, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
Pretty awesome that this confirms to the json schema definition. Could you link the Java generator ? |
the pom.xml that generates java model from json schema https://github.com/gorkem/java-language-server/blob/master/server/lang-protocol-generator/pom.xml |
@gorkem: nice work. Thanks a lot. Before we merge this in I have a couple of questions / suggestions:
"MessageType": {
"type": "string"
}, but in TS /**
* The message type
*/
export enum MessageType {
/**
* An error message.
*/
Error = 1,
/**
* A warning message.
*/
Warning = 2,
/**
* An information message.
*/
Info = 3,
/**
* A log message.
*/
Log = 4
}
Did you generate the file with a tool. May be we can tweak it a little for our purpose. |
@gorkem and you need to sign the CLA (see #40 (comment)). Otherwise I can't merge it in. |
@gorkem are you going to sign the CLA and work on the things @dbaeumer mentioned? We would like to use this as the source for our code generator in https://github.com/TypeFox/ls-api |
@svenefftinge Unfortunately, we needed a CCLA so this is waiting on that. |
@dbaeumer I only know about the other way: https://github.com/YousefED/typescript-json-schema |
@@ -0,0 +1,1216 @@ | |||
{ | |||
"type": "object", |
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 this really be an object? Shouldn't the schema only define all structures?
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.
It is something that the generator tool created. We should remove it going forward.
A Json schema that is mostly generated from protocol.d.json
@dbaeumer I should have a CLA now, is there a way to check it has gone in effect? |
@gorkem usually the cla label is updated by the cla bot. Let me see what is missing here. |
Looks like closing and reopening should trigger a revalidate. Let me see if it works. |
OK. Here we go. Instead of simply applying this PR I think we also need to work on a tool chain to generate the schema from the protocol.ts file here: https://github.com/Microsoft/vscode-languageserver-node/blob/master/client/src/protocol.ts Maintaining the schema by hand will be a nightmare and I don't want to make json schema the master :-) @gorkem which tool did you use to generate the JSON schema. |
I have used typescript-json-schema . It needs to be massaged in some cases but works most of the time. I also have a Gruntfile I used that automates things which I can share. |
Great. Can you attach it here? |
I will close the PR. If we want a json schema we should invest in some automation. |
A Json schema that is mostly generated from protocol.d.json