langserver.py: Update to use jsonrpc by ksdme · Pull Request #32 · coala/coala-vs-code · GitHub

Updates coala-vs-code to use jsonrpc module from python-language-server (palantir/python-language-server#32).

Closes #27

Thanks for your contribution! Could you please fix the test failures?

I am already working on it.

The jsonrpc behave tests are updated. The test has remained unchanged. Related to coala#27, coala#32

Please follow the contribution guide here #31 (comment)

The jsonrpc behave tests have been updated. The scope of the test has remained unchanged. Related to coala#27, coala#32

The behave tests have been updated to reflect the new jsonrpc handling mechanism. The scope of the tests remains unchanged. Related to coala#27, coala#32

I have only updated the tests as of now, I am working on improving the coverage and fix a minor logging issue during running testing.

discard this change. create a newcomer bug about adding style linting rules for this file.

Noted.

Create a newcomer issue about this, and remove it from your PR. it has nothing to do with jsonrpc.

Also a change like this should be done by adding a new line, so the origin of existing line doesnt change unnecessarily.

Noted.

call this logger

we dont want commented out code.

If there is extra stuff to be done, create an issue after this is merged

A clean up issue, I just wanted to have a reference to the previous code to cross verify.

this seems to be a pointless change

I would prefer that we did not copy other code into the repository. For example, I can not easily check to see if you modified a line from the origin.

While there are some additional dependencies at https://github.com/palantir/python-language-server/blob/develop/setup.py#L34 , could we not require that it is installed, and then use the jsonrpc they provide? If that is not possible, maybe we can coordinate with @palantir to split that out into a sharable library.

This tool is still a prototype, so it can have additional install steps , and keeping the origin of source code clear.

It also means that additional tests for the jsonrpc would be contributed to https://github.com/palantir/python-language-server , so everyone benefits.

I will follow your recommendations, python-lang-server will be added as a dependency.

why does it need renaming from f to file? It causes all sorts of other unnecessary changes to appear in this PR, slowing down the review.

is that related to the issue that this PR is intending to solve.

'f' didn't seem to be a semantic name. My intention was to improve readability, retaining such occurrences could indicate tolerance for those practices and could even mean encouragement.

I think the re-naming is justified, do you still recommend this particular change?

Of course I do. it is not related to using jsonrpc. Create a new PR for other changes you wish to make.

See #35

sys.exit?

Noted.

removing this (and other logging enhancements) should be done as a separate PR. I've assigned #36 to you, as you've already done a lot of this.

Noted.

The jsonrpc behave tests have been updated. The scope of the test has remained unchanged. Related to coala#27, coala#32

The behave tests have been updated to reflect the new jsonrpc handling mechanism. The scope of the tests remains unchanged. Related to coala#27, coala#32

This change should be a new PR.

Ideally create a new issue showing why this change is needed.

move this code out to a PR for #48

As far as I can see, after removing all the unrelated features, there should only be one commit in this PR. fwiw, updating tests should be part of the atomic change of modifying the code.

@ksdme Thanks for your contribution! I think we could try to add some tests.

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp_evza00r/coala_langserver/langserver.py
+++ b/tmp/tmp_evza00r/coala_langserver/langserver.py
@@ -14,7 +14,7 @@
 
 
 class _StreamHandlerWrapper(socketserver.StreamRequestHandler, object):
-    """A wrapper class that is used to construct a custom handler class."""
+    '""A wrapper class that is used to construct a custom handler class.""'
 
     delegate = None

E265 block comment should start with '# '

Origin: PycodestyleBear (E265), Section: autopep8.

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp7wx9j6yx/coala_langserver/langserver.py
+++ b/tmp/tmp7wx9j6yx/coala_langserver/langserver.py
@@ -111,7 +111,7 @@
     try:
         server = socketserver.TCPServer((bind_addr, port), wrapper_class)
     except Exception as e:
-        log("Fatal Exception: {}".format(e))
+        log('Fatal Exception: {}'.format(e))
         sys.exit(1)
     try:
         log('Serving %s on (%s, %s)', handler_class.__name__, bind_addr, port)

Comment on f253090.

Shortlog of the HEAD commit contains 60 character(s). This is 10 character(s) longer than the limit (60 > 50).

Origin: GitCommitBear, Section: commit.

Comment on 45df0f8, file coala_langserver/langserver.py, line 126.

Line is longer than allowed. (85 > 79)

Origin: LineLengthBear, Section: linelength.

Increases the codecov target to 95%. Related to coala#27

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 63.

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpc942a8vq/tests/server.features/steps/jsonrpc_steps.py
+++ b/tmp/tmpc942a8vq/tests/server.features/steps/jsonrpc_steps.py
@@ -60,7 +60,8 @@
 @given('the JSONRPC2Connection instance')
 def step_impl(context):
     context.f = tempfile.TemporaryFile()
-    context.jsonConn = JSONRPC2Connection(conn=TCPReadWriter(context.f, context.f))
+    context.jsonConn = JSONRPC2Connection(
+        conn=TCPReadWriter(context.f, context.f))
 
 
 @when('I write a request to the JSONRPC2Connection with id')

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 10.

E501 line too long (82 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 63.

E501 line too long (83 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 10.

Line is longer than allowed. (82 > 79)

Origin: LineLengthBear, Section: linelength.

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 63.

Line is longer than allowed. (83 > 79)

Origin: LineLengthBear, Section: linelength.

trailing comma

still not done?

trailing comma ; not likely to be needed any time soon, but likely doesnt hurt

log isnt useful inside the try.

but, it is a new log call, and redundant due to existing Accepting TCP connections .. log

I have to retain this log compared to the other one because the tests watch this log call to trigger sending requests.

new log call, also redundant.

For the reasons mentioned in the previous change request and maintaining uniformity I think I'll have to retain this log and remove the other redundant log call.

ok.

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 63.

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpc942a8vq/tests/server.features/steps/jsonrpc_steps.py
+++ b/tmp/tmpc942a8vq/tests/server.features/steps/jsonrpc_steps.py
@@ -60,7 +60,8 @@
 @given('the JSONRPC2Connection instance')
 def step_impl(context):
     context.f = tempfile.TemporaryFile()
-    context.jsonConn = JSONRPC2Connection(conn=TCPReadWriter(context.f, context.f))
+    context.jsonConn = JSONRPC2Connection(
+        conn=TCPReadWriter(context.f, context.f))
 
 
 @when('I write a request to the JSONRPC2Connection with id')

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 10.

E501 line too long (82 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 63.

E501 line too long (83 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 10.

Line is longer than allowed. (82 > 79)

Origin: LineLengthBear, Section: linelength.

Comment on de0098b, file tests/server.features/steps/jsonrpc_steps.py, line 63.

Line is longer than allowed. (83 > 79)

Origin: LineLengthBear, Section: linelength.

no, a trailing comma is for allowing new additions without modifications to exiting lines. hence this , is not a trailing comma.

{'DELEGATE_CLASS': handler_class},

ok.

still not done?

def m_initialize(self, **params)

then nothing below needs to be modified, except removal of params = request['params']

sorry I didnt notice this earlier.

this helps show the similarities of the two jsonrpc's , making differences more clear

likewise use **params to avoid changes below

This updates the project to use jsonrpc module from python-language-server. Closes coala#27

ack bd12657

ack de0098b

@gitmate-bot ff

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

Automated fastforward with GitMate.io was successful! 🎉

🎉

ksdme added a commit to ksdme/coala-vs-code that referenced this pull request Mar 28, 2018

Updates tests to json-rpc

77a828a

The jsonrpc behave tests are updated. The test has remained unchanged. Related to coala#27, coala#32

ksdme force-pushed the jsonrpc-dev branch from 77a828a to 63b9f75 Compare March 28, 2018 10:08

ksdme added a commit to ksdme/coala-vs-code that referenced this pull request Mar 28, 2018

Updates tests to jsonrpc

59605c2

The jsonrpc behave tests have been updated. The scope of the test has remained unchanged. Related to coala#27, coala#32

ksdme added a commit to ksdme/coala-vs-code that referenced this pull request Mar 28, 2018

langserver_steps.py: Updates behave tests

63b9f75

The behave tests have been updated to reflect the new jsonrpc handling mechanism. The scope of the tests remains unchanged. Related to coala#27, coala#32

jayvdb suggested changes Mar 29, 2018

View reviewed changes

jayvdb added the process/wip label Mar 29, 2018

ksdme force-pushed the jsonrpc-dev branch from 63b9f75 to b3b46d3 Compare April 3, 2018 12:12

ksdme added a commit to ksdme/coala-vs-code that referenced this pull request Apr 3, 2018

Updates tests to jsonrpc

601463d

The jsonrpc behave tests have been updated. The scope of the test has remained unchanged. Related to coala#27, coala#32

ksdme added a commit to ksdme/coala-vs-code that referenced this pull request Apr 3, 2018

langserver_steps.py: Updates behave tests

b3b46d3

The behave tests have been updated to reflect the new jsonrpc handling mechanism. The scope of the tests remains unchanged. Related to coala#27, coala#32

ksdme force-pushed the jsonrpc-dev branch from b3b46d3 to 9426388 Compare April 4, 2018 03:23

jayvdb reviewed Apr 4, 2018

View reviewed changes

ksdme force-pushed the jsonrpc-dev branch from 9426388 to 9248c90 Compare April 5, 2018 06:16

gitmate-bot added process/pending review size/L difficulty/low importance/low and removed process/wip labels Apr 5, 2018

gitmate-bot assigned ksdme Apr 5, 2018

gitmate-bot reviewed Apr 5, 2018

View reviewed changes

jayvdb changed the title Updated to use jsonrpc langserver.py: Update to use jsonrpc May 11, 2018

.codecov.yml: Increase test coverage target

de0098b

Increases the codecov target to 95%. Related to coala#27

ksdme force-pushed the jsonrpc-dev branch from ead85e3 to 2013904 Compare May 11, 2018 09:57

jayvdb suggested changes May 11, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels May 11, 2018

ksdme force-pushed the jsonrpc-dev branch from 2013904 to 92ad7bc Compare May 11, 2018 10:47

gitmate-bot added process/pending review and removed process/wip labels May 11, 2018

jayvdb suggested changes May 11, 2018

View reviewed changes

jayvdb suggested changes May 12, 2018

View reviewed changes

Update to jsonrpc lib

bd12657

This updates the project to use jsonrpc module from python-language-server. Closes coala#27

jayvdb approved these changes May 12, 2018

View reviewed changes

jayvdb mentioned this pull request Jun 6, 2018

(Phase 1) Project wide refactor and generalization coala/coala-ls#2

Open

                       params = {
                           'uri': 'file://{0}'.format(path),
-                          'diagnostics': _diagnostics,
+                          'diagnostics': [] if diagnostics is None else diagnostics,

-                      # s = LangServer(conn=ReadWriter(sys.stdin, sys.stdout))
-                      # s.listen()
-                      pass
+                      stdin, stdout = _binary_stdio()

+              from pyls.jsonrpc.endpoint import Endpoint
+              from pyls.jsonrpc.dispatchers import MethodDispatcher
+              from pyls.jsonrpc.streams import JsonRpcStreamReader
+              from pyls.jsonrpc.streams import JsonRpcStreamWriter

+                          'capabilities': {},
+                      },
+                      'id': 1,
+                      'jsonrpc': '2.0'

+                          'capabilities': {},
+                      },
+                      'id': 1,
+                      'jsonrpc': '2.0'

               You'll need python version 3.5 or greater, run `pip3 install -r requirements.txt` to install the requirements, and run `python3 langserver-python.py --mode=tcp --addr=2087` to start a local languager server listening at port 2087.
-              Then you should update the `./vscode-client/src/extension.ts` to make client in TCP mode.
+              Then you should update the `./vscode-client/src/extension.ts` to make client in TCP mode.

               ```
-              To try it in [Visual Studio Code](https://code.visualstudio.com), open ./vscode-client in VS Code and turn to debug view, launch the extension.
+              To try it in [Visual Studio Code](https://code.visualstudio.com), open ./vscode-client in VS Code and turn to debug view, launch the extension. If the langserver fails try disabling the GitCommitBear.

               import json
+              import logging
+              log = logging.getLogger(__name__)

+                  def start(self):
+                      self._jsonrpc_stream_reader.listen(self._endpoint.consume)
+                  # def handle(self, _id, request):

		@@ -0,0 +1,35 @@
		# Copyright 2018 Palantir Technologies, Inc.

+                  """
+                  PY3K = sys.version_info >= (3, 0)
+                  if PY3K:

+              import logging
-              from .jsonrpc import JSONRPC2Connection, ReadWriter, TCPReadWriter
-              from .log import log

-                          self.write_response(request['id'], resp)
-                  def serve_initialize(self, request):
+                  #TODO: didChange

+                  wrapper_class = type(
+                      handler_class.__name__ + 'Handler',
+                      (_StreamHandlerWrapper,),
+                      {'DELEGATE_CLASS': handler_class}

+              @enforce_signature
+              def start_io_lang_server(handler_class: LangServer, rstream, wstream):
+                  log('Starting {} IO language server'.format(handler_class.__name__))

                       }
-                  def serve_did_save(self, request):
+                  def m_text_document__did_save(self, textDocument=None, **kargs):

langserver.py: Update to use jsonrpc by ksdme · Pull Request #32 · coala/coala-vs-code · GitHub

Uh oh!

langserver.py: Update to use jsonrpc - #32

langserver.py: Update to use jsonrpc#32gitmate-bot merged 2 commits intocoala:mastercoala/coala-vs-code:masterfrom ksdme:jsonrpc-devksdme/coala-vs-code:jsonrpc-devCopy head branch name to clipboard

Conversation

ksdme commented Mar 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaocegege commented Mar 27, 2018

Uh oh!

ksdme commented Mar 27, 2018

Uh oh!

gaocegege commented Mar 28, 2018

Uh oh!

ksdme commented Mar 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jayvdb commented Apr 4, 2018

Uh oh!

gaocegege commented Apr 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitmate-bot commented Apr 5, 2018

Uh oh!

gitmate-bot commented Apr 5, 2018

Uh oh!

gitmate-bot commented May 11, 2018

Uh oh!

gitmate-bot commented May 11, 2018

Uh oh!

gitmate-bot commented May 11, 2018

Uh oh!

gitmate-bot commented May 11, 2018

Uh oh!

langserver.py: Update to use jsonrpc#32
gitmate-bot merged 2 commits into
coala:masterfrom
ksdme:jsonrpc-dev

ksdme commented Mar 26, 2018 •
edited
Loading

ksdme May 11, 2018 •
edited
Loading