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

Fix documentLink/resolve by introducing a data field #336

Merged
merged 1 commit into from Apr 26, 2018

Conversation

@rcjsuen
Copy link
Contributor

commented Apr 12, 2018

The current definition of a DocumentLink makes resolving a link with documentLink/resolve essentially impossible. With only a range field available, it is not possible for the server to deduce what the originating content is as it does not have any information as to which text document the link is for.

To correct this, a new data field has been introduced to DocumentLink that servers can fill in with some whatever it needs when initially fulfilling the textDocument/documentLink request. This way, a subsequent documentLink/resolve request will have the necessary contextual information available to resolve the link and return back a URI to use in the target field.

Fix documentLink/resolve by introducing a data field
The current definition of a DocumentLink makes resolving a link with
documentLink/resolve essentially impossible. Having only a range
field filled out is not enough information for a language server to
deduce what content or file the link is about. To correct this, a new
data field has been introduced to DocumentLink that servers can fill
in with some data on the initial fulfillment of a
textDocument/documentLink request so that subsequent
documentLink/resolve requests will have some contextual information
available to resolve the link.

Signed-off-by: Remy Suen <remy.suen@gmail.com>

@rcjsuen rcjsuen force-pushed the rcjsuen:link-data branch from 88e0cf1 to 0b2de05 Apr 16, 2018

@dbaeumer dbaeumer merged commit 4c66383 into microsoft:master Apr 26, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
@dbaeumer

This comment has been minimized.

Copy link
Member

commented Apr 26, 2018

@rcjsuen can you please update the spec. E.g. have

	/**
	 * A data entry field that is preserved on a document link between a
	 * DocumentLinkRequest and a DocumentLinkResolveRequest.
	 */
	data?: any

in there as well.

@rcjsuen

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2018

Thank you for merging in the PR, Dirk. I've opened microsoft/language-server-protocol#464 to add the data property to the spec.

@dbaeumer

This comment has been minimized.

Copy link
Member

commented Apr 26, 2018

@rcjsuen thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.