Path mapping Part II

[rwols]

This adds a ClientConfig argument to most of the functions in views.py so that
we can translate from local paths to remote paths.

Some functions are used by helper packages so we have to put the argument at the
end and make it optional. Version 1.3 should make them mandatory.

[rwols]

I have written a Dockerfile in tests/pyright-docker, but I cannot get it to work.

I built this Dockerfile with

docker build . -t pyright

My client configuration is:

		"docker-pyright": {
			"command": [
				"docker",
				"run",
				"--rm",
				"--interactive",
				"--attach", "stdin",
				"--attach", "stdout",
				"--attach", "stderr",
				"--mount",
				"type=bind,src=${folder},dst=/workspace",
				"pyright",
			],
			"selector": "source.python",
			"enabled": true,
			"path_maps": [
				{
					"local": "${folder}",
					"remote": "/workspace"
				}
			]
		},

It looks like the stdin pipe is receiving no bytes at some point. Does anyone have any ideas about what might go wrong here?

[rchl]

It looks like the stdin pipe is receiving no bytes at some point. Does anyone have any ideas about what might go wrong here?

The pyright process kills itself from this place: https://github.com/microsoft/vscode-languageserver-node/blob/44ab977c6a54407d725048e33201f7ad9899cae9/server/src/node/main.ts#L80-L92

It periodically checks if the parent process is alive with process.kill(pid, 0) (documentation) and apparently in this special case it's deemed dead.

It checks the processId that we pass in initialize request so I guess we are passing the wrong one in this case. We would need to pass the parent process from inside the container somehow.

[rchl]

And here is how I've debugged it:

Added --inspect-brk argument in Dockerfile's RUN command:

FROM node:15
RUN npm install -g pyright
CMD ["node", "--inspect-brk=0.0.0.0:9229", "/usr/local/lib/node_modules/pyright/langserver.index.js", "--stdio"]

Changed the docker run command to expose the port with -p 9229:9229:

		"docker-pyright": {
			"command": [
				"docker",
				"run",
				"--rm",
				"--interactive",
				"--attach", "stdin",
				"--attach", "stdout",
				"--attach", "stderr",
				"-p", "9229:9229",
				"--mount",
				"type=bind,src=${folder},dst=/workspace",
				"pyright",
			],
			"selector": "source.python",
			"enabled": true,
			"path_maps": [
				{
					"local": "${folder}",
					"remote": "/workspace"
				}
			]
		},

This makes the pyright node process block on start until it's connected to through a debugger so then open developer tool in Chrome and a green "Node" icon should appear in its top-left corner. Clicking it will connect to pyright and let it continue.

[rchl]

Or we can just not pass processId.

[rchl]

Another issue that made many requests hang (send no response) for me was the lack of settings: { ... } in the client config. It must be a non-empty object or otherwise, pyright will not initialize the workspace properly due to us not sending workspace/didChangeConfiguration.

[rwols]

Some amazing detective work again. I say just omit the processId whenever there's a path mapping.

[rwols]

Also you're right about the settings. It shows that we should eventually end up with only helper packages, as the set up can become so complex with so many failure points.

I have instead now changed Packages/User/LSP-pyright.sublime-settings to this:

// Settings in here override those in "LSP-pyright/LSP-pyright.sublime-settings"
{
    "dev_environment": "sublime_text",
    "settings": {
        "python.analysis.stubPath": "./stubs"
    },
    "command": [
        "docker",
        "run",
        "--rm",
        "--interactive",
        "--attach",
        "stdin",
        "--attach",
        "stdout",
        "--attach",
        "stderr",
        "--mount",
        "type=bind,src=${folder},dst=/workspace",
        "pyright",
    ],
    "path_maps": [
        {
            "local": "${folder}",
            "remote": "/workspace"
        }
    ]
}

and everything seems to work okay! I'll have to dig through a few cases like diagnostics to see if paths are translated correctly there.

[rchl]

Some amazing detective work again. I say just omit the processId whenever there's a path mapping.

You can also see above the code I've linked (here https://github.com/microsoft/vscode-languageserver-node/blob/44ab977c6a54407d725048e33201f7ad9899cae9/server/src/node/main.ts#L42-L61) that there is an alternative code there that sets up a parent process watcher based on --clientProcessId argument. I think that's made for such cases as this one here. It should be possible to set that argument from inside of the Dockerfile (from some wrapper script that would start pyright).

But I'm not sure how useful it is to have a watcher within a docker container. I would think that when the parent process dies it will just close the node process automatically.

[rwols]

Yes, whatever container daemon is hosting the language server is responsible for killing it.

[rchl]

I'd suggest 14 because in the Node world, the even-numbered versions become stable and supported versions.

[rchl]

Just seeing all those similar changes makes me want to look into ways to refactor this.

Could create some kind of abstraction object (maybe TextDocument to match the spec) that would include methods to get text_document_identifier (which is what those two arguments seem to be needed for typically) and the path mapping function.

Could be initialized with filename_or_view and the config (or even session).

Just a wild idea...

[rwols]

Would that work for the stuff in plugin/references.py (for example, or in plugin/rename.py) as well?

[rchl]

I have no idea if it would work in general ;)

[rwols]

Unfortunately I’m running into quite a mess when dealing with diagnostics and code actions. I should separate that into another PR.

[rwols]

I don't think I'll finish this. The better solution is creating an interface for a virtual file system and taking care of remote language servers.