-
Notifications
You must be signed in to change notification settings - Fork 70
Make updates to be spec-compliant to the language server protocol #8
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,3 @@ | ||
!.eslintrc.js | ||
**/node_modules/** | ||
**/VendorLib/** | ||
**/flow-typed/** | ||
pkg/nuclide-external-interfaces/1.0/simple-text-buffer.js |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,13 @@ | |
|
||
_This is currently in technical preview. We welcome your feedback and suggestions._ | ||
|
||
GraphQL Language Service provides an interface for building GraphQL language services for IDEs. Currently supported features include: | ||
- Diagnostics (GraphQL syntax linting/validations) | ||
- Autocomplete suggestions | ||
GraphQL Language Service provides an interface for building GraphQL language service for IDEs. | ||
|
||
A subset of supported features of GraphQL language service and GraphQL language server implementation are both specification-compliant to [Microsoft's Language Server Protocol](https://github.com/Microsoft/language-server-protocol), and will be developed to fully support the specification in the future. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sentence is hard to read. "GraphQL language service and GraphQL language server implementation" is confusing. Could we simplify this to:
|
||
|
||
Currently supported features include: | ||
- Diagnostics (GraphQL syntax linting/validations) (**spec-compliant**) | ||
- Autocomplete suggestions (**spec-compliant**) | ||
- Hyperlink to fragment definitions | ||
- Outline view support for queries | ||
|
||
|
@@ -61,31 +65,42 @@ The node executable contains several commands: `server` and a command-line langu | |
Improving this list is a work-in-progress. | ||
|
||
``` | ||
Usage: graphql <command> | ||
|
||
GraphQL Language Service Command-Line Interface. | ||
Usage: bin/graphql.js <command> <file> | ||
[-h | --help] | ||
[-c | --config] {configPath} | ||
[-c | --configDir] {configDir} | ||
[-t | --text] {textBuffer} | ||
[-f | --file] {filePath} | ||
[-s | --schema] {schemaPath} | ||
|
||
|
||
Options: | ||
-h, --help Show help [boolean] | ||
-c, --config GraphQL Config file path (.graphqlrc). | ||
Will look for the nearest .graphqlrc file if omitted. | ||
-h, --help Show help [boolean] | ||
-t, --text Text buffer to perform GraphQL diagnostics on. | ||
Will defer to --file option if omitted. | ||
This option is always honored over --file option. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd collapse these two into a single line:
|
||
[string] | ||
-t, --text Text buffer to perform GraphQL lint on. | ||
Will defer to --file option if omitted. | ||
This option is always honored over --file option. | ||
-f, --file File path to perform GraphQL diagnostics on. | ||
Will be ignored if --text option is supplied. | ||
[string] | ||
-f, --file File path to perform GraphQL lint on. | ||
Will be ignored if --text option is supplied. | ||
--row A row number from the cursor location for GraphQL | ||
autocomplete suggestions. | ||
If omitted, the last row number will be used. | ||
[number] | ||
--column A column number from the cursor location for GraphQL | ||
autocomplete suggestions. | ||
If omitted, the last column number will be used. | ||
[number] | ||
-c, --configDir A directory path where .graphqlrc configuration object is | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sentence is missing a trailing period, but I think we can make it simpler anyway:
|
||
Walks up the directory tree from the provided config | ||
directory, or the current working directory, until | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "until" → "until a" |
||
.graphqlrc is found or the root directory is found. | ||
[string] | ||
-s, --schema a path to schema DSL file | ||
-s, --schemaPath a path to schema DSL file | ||
[string] | ||
|
||
At least one command is required. | ||
Commands: "server, lint, autocomplete, outline" | ||
Commands: "server, validate, autocomplete, outline" | ||
``` | ||
|
||
## Architectural Overview | ||
|
@@ -112,84 +127,14 @@ The IDE server should manage the lifecycle of the GraphQL server. Ideally, the I | |
|
||
### Server Interface | ||
|
||
The server sends/receives RPC messages to/from the IDE server to perform language service features. The details for the RPC message format are described below: | ||
|
||
``` | ||
/** | ||
* The JSON message sent from the IDE server should have the following structure: | ||
* { | ||
* protocol: 'graphql-protocol', | ||
* id: number, | ||
* method: string, // one of the function names below, e.g. `getDiagnostics` | ||
* args: { | ||
* query?: string, | ||
* position?: Point, | ||
* filePath?: Uri, | ||
* } | ||
* } | ||
*/ | ||
// Diagnostics (lint/validation) | ||
export type GraphQLDiagnosticMessage = { | ||
name: string, | ||
type: string, | ||
text: string, | ||
range: atom$Range, | ||
filePath: string, | ||
}; | ||
|
||
export function getDiagnostics( | ||
query: string, | ||
filePath: Uri, | ||
) : Promise<Array<GraphQLDiagnosticMessage>> { | ||
throw new Error('RPC stub'); | ||
} | ||
|
||
// Autocomplete Suggestions (typeahead) | ||
export type GraphQLAutocompleteSuggestionType = { | ||
text: string, | ||
typeName: ?string, | ||
description: ?string, | ||
}; | ||
|
||
export function getAutocompleteSuggestions( | ||
query: string, | ||
position: atom$Point, | ||
filePath: Uri, | ||
) : Promise<Array<GraphQLAutocompleteSuggestionType>> { | ||
throw new Error('RPC stub'); | ||
} | ||
GraphQL Language Server uses [JSON-RPC](http://www.jsonrpc.org/specification) to communicate with the IDE servers to perform language service features. The language server currently supports two communication transports: Stream (stdio) and IPC. For IPC transport, the reference guide to be used for development is [the language server protocol](https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md) documentation. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could delete "to perform language service features" (it's already implied). Suggest: "the language server protocol" → "Microsoft's language server protocol" to make things clearer. |
||
|
||
// Definitions (hyperlink) | ||
export type Definition = { | ||
path: Uri, | ||
position: Point, | ||
range?: Range, | ||
id?: string, | ||
name?: string, | ||
language: string, | ||
projectRoot?: Uri, | ||
}; | ||
|
||
export type DefinitionQueryResult = { | ||
queryRange: Array<Range>, | ||
definitions: Array<Definition>, | ||
}; | ||
|
||
export function getDefinition( | ||
query: string, | ||
position: atom$Point, | ||
filePath: Uri, | ||
): Promise<DefinitionQueryResult> { | ||
throw new Error('RPC stub'); | ||
} | ||
For each transports, there is a slight difference between both JSON message format, especially in how the methods to be invoked are defined - below are the currently supported methods for each transports (will be updated as progresses are made): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "each transports" → " each transport" (in two places) "difference between both JSON message format" → "difference in JSON message format" "progresses are" → "progress is" |
||
|
||
// Outline view | ||
export function getOutline(query: string): Promise<Outline> { | ||
throw new Error('RPC stub'); | ||
} | ||
|
||
// Disconnect signal - gracefully terminate the connection on IDE exit | ||
export function disconnect(): void { | ||
throw new Error('RPC stub'); | ||
} | ||
``` | ||
| | Stream | IPC | | ||
| -------------------:|------------------------------|-----------------------------------| | ||
| Diagnostics | `getDiagnostics` | `textDocument/publishDiagnostics` | | ||
| Autocompletion | `getAutocompleteSuggestions` | `textDocument/completion` | | ||
| Outline | `getOutline` | Not supported yet | | ||
| Go-to definition | `getDefinition` | Not supported yet | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can we make the trailing bars line up? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That one's a little tricky as the lines are too long I think - markdown doesn't care for it though, so I think we're good here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I know it will render correctly to HTML, but one of the ideas of Markdown is that the plain-text should look good too. I personally think its ok if the lines here are long (in fact, we're already not hard-wrapping the lines in the file, so we already have lots of long lines). |
||
| File Events | Not supported yet | `didOpen/didClose/didSave/didChange` events | |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ const {argv} = yargs | |
'GraphQL Language Service Command-Line Interface.\n' + | ||
'Usage: $0 <command> <file>\n' + | ||
' [-h | --help]\n' + | ||
' [-c | --config] {configPath}\n' + | ||
' [-c | --configDir] {configDir}\n' + | ||
' [-t | --text] {textBuffer}\n' + | ||
' [-f | --file] {filePath}\n' + | ||
' [-s | --schema] {schemaPath}\n', | ||
|
@@ -52,6 +52,14 @@ const {argv} = yargs | |
'If omitted, the last column number will be used.\n', | ||
type: 'number', | ||
}) | ||
.option('c', { | ||
alias: 'configDir', | ||
describe: 'A directory path where .graphqlrc configuration object is\n' + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comments above on README. |
||
'Walks up the directory tree from the provided config directory, or ' + | ||
'the current working directory, until .graphqlrc is found or ' + | ||
'the root directory is found.\n', | ||
type: 'string', | ||
}) | ||
.option('s', { | ||
alias: 'schemaPath', | ||
describe: 'a path to schema DSL file\n', | ||
|
@@ -62,7 +70,7 @@ const command = argv._.pop(); | |
|
||
switch (command) { | ||
case 'server': | ||
startServer(argv.config.trim()); | ||
startServer(argv.configDir); | ||
break; | ||
default: | ||
client(command, argv); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,26 @@ const CONFIG_LIST_NAME = 'build-configs'; | |
const SCHEMA_PATH = 'schema-file'; | ||
const CUSTOM_VALIDATION_RULES_MODULE_PATH = 'custom-validation-rules'; | ||
|
||
/** | ||
* Finds a .graphqlrc configuration file, and returns null if not found. | ||
* If the file isn't present in the provided directory path, walk up the | ||
* directory tree until the file is found or it reaches the root directory. | ||
*/ | ||
export async function findGraphQLConfigDir(dirPath: Uri): Promise<?string> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this |
||
let currentPath = path.resolve(dirPath); | ||
let filePath; | ||
while (currentPath.length > 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like it will fail if currentPath tops out at "/" (length 1), so we'd fail to find "/.graphqlrc", wouldn't we? (I know this is likely, but...) |
||
filePath = path.join(currentPath, '.graphqlrc'); | ||
if (fs.existsSync(filePath)) { | ||
break; | ||
} | ||
|
||
currentPath = path.dirname(currentPath); | ||
} | ||
|
||
return filePath ? currentPath : null; | ||
} | ||
|
||
export async function getGraphQLConfig(configDir: Uri): Promise<GraphQLRC> { | ||
const rawGraphQLConfig = await new Promise((resolve, reject) => | ||
fs.readFile( | ||
|
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 still be "for building GraphQL language services for IDEs" (plural services), because grammar. Or if you prefer, "for powering GraphQL language features in IDEs".