Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@vhakulinen
Copy link

Include range property for the textDocument/hover response: https://microsoft.github.io/language-server-protocol/specification#textDocument_hover.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/python-language-server, @vhakulinen! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

Copy link
Contributor

@gatesn gatesn left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, really nice. Couple of small comments then should be good to go


return {
'contents': _utils.format_docstring(definitions[0].docstring()) or "",
'range': pos,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you inline pos here

@@ -1,5 +1,6 @@
# Copyright 2017 Palantir Technologies, Inc.
import logging
import json
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused?

if position['line'] >= len(self.lines):
return None

line = self.lines[position['line']]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test for this method explicitly?

@gatesn gatesn changed the base branch from develop to ngates/range-hover February 28, 2019 08:44
@gatesn
Copy link
Contributor

gatesn commented Feb 28, 2019

Copying over to a local branch so I can finish up the changes

@gatesn gatesn merged commit d318fb3 into palantir:ngates/range-hover Feb 28, 2019
@vhakulinen
Copy link
Author

Sorry, I completely forgot this while doing other stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.