fix empty command behavior on windows

[AlexeyBond]

Plugin fails with "list index out of range" error when running on windows and a server is configured with empy command and non-empty tcp_port.

Looks like regression of #2118, but it also doesn't look like it was really fixed.

[predragnikolic]

Hello @AlexeyBond ,

Can you paste the clients configuration that you have in LSP.sublime-settings that can reproduce the issue?

[AlexeyBond]

Can you paste the clients configuration that you have in LSP.sublime-settings that can reproduce the issue?

LSP.sublime-settings contains the following configuration:

// Settings in here override those in "LSP/LSP.sublime-settings"
{
	"clients": {
		"test-lsp": {
			"selector": "source.lisp",
			"tcp_port": 9257,
			"enabled": true
		},
	},
}

I'm running an example server from tower-lsp on port 9257.

[predragnikolic]

the bug is caused by the _fixup_startup_args function

def _fixup_startup_args(args: List[str]) -> Any:
    startupinfo = None
    if sublime.platform() == "windows":
        # ... this line bellow causes the Error because, args is `[]`
        executable_arg = args[0]

start_subprocess will be called in 2 cases:

-   startupinfo = _fixup_startup_args(config.command)


    def start_subprocess() -> subprocess.Popen:
+       startupinfo = _fixup_startup_args(config.command)
        return _start_subprocess(config.command, stdin, stdout, subprocess.PIPE, startupinfo, config.env, cwd)

    if config.listener_socket:
        assert isinstance(config.tcp_port, int) and config.tcp_port > 0
        process, sock, reader, writer = _await_tcp_connection(
            config.name, config.tcp_port, config.listener_socket, start_subprocess) # <- Case 1
    else:
        if config.command:
            process = start_subprocess() # <- Case 2 

In this PR the _fixup_startup_args will not be called in Case 2 anymore,
so the bug in that case will be fixed.

But in Case 1, when start_subprocess is called, the _fixup_startup_args will also be called, and the command will still be None. So we still have a bug there.

IMO, a better option would be to fix the _fixup_startup_args function:

def _fixup_startup_args(args: List[str]) -> Any:
+     if not args:
+         return None
    startupinfo = None
    if sublime.platform() == "windows":
        startupinfo = subprocess.STARTUPINFO()  # type: ignore
        startupinfo.dwFlags |= subprocess.SW_HIDE | subprocess.STARTF_USESHOWWINDOW  # type: ignore
        executable_arg = args[0]
        _, ext = os.path.splitext(executable_arg)
        if len(ext) < 1:
            path_to_executable = shutil.which(executable_arg)
            # what extensions should we append so CreateProcess can find it?
            # node has .cmd
            # dart has .bat
            # python has .exe wrappers - not needed
            for extension in ['.cmd', '.bat']:
                if path_to_executable and path_to_executable.lower().endswith(extension):
                    args[0] = executable_arg + extension
                    break
    return startupinfo

[AlexeyBond]

Looks like something else is wrong with case 1: it doesn't make sense to start a subprocess with empty command. And _start_subprocess will fail when first argument is empty:

def _start_subprocess(
    args: List[str],
# ...
) -> subprocess.Popen:
    debug("starting {} in {}".format(args, cwd if cwd else os.getcwd()))
    process = subprocess.Popen( # <- Will throw if args is empty
        args=args,
        # ...
    )
    _subprocesses.add(process)
    return process

So, even with check added in _fixup_startup_args, it will still fail, but in _start_subprocess.

[AlexeyBond]

I decided to split _await_tcp_connection into two functions:

• _await_client_connection - waits for client to connect to listener socket
• _start_client_process_and_await_connection - starts client process and simulteniously calls _await_client_connection

and call them depending on presence of command in configuration when language server is running as TCP client.

Just in case, configuration to reproduce this scenario:

// Settings in here override those in "LSP/LSP.sublime-settings"
{
	"clients": {
		"test-lsp": {
			"selector": "source.lisp",
			"tcp_port": -9257,
			"enabled": true
		},
	},
}

Language server startup will fail with timeout error if language server doesn't try to connect to editor port. Previously it would also fail with "list index out of range".

I have tested these changes with manually launched language server running in client and server modes. But haven't yet tested with server process launched by editor (non-empty command).

UPD: Tested in all scenarios - with and without command, in client and server mode. Seems to work fine in all cases.