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

refact-lsp process handling #3

Closed
olegklimov opened this issue Feb 7, 2024 · 9 comments
Closed

refact-lsp process handling #3

olegklimov opened this issue Feb 7, 2024 · 9 comments

Comments

@olegklimov
Copy link
Contributor

process_server_errors() approach is probably not a good idea (a thread?), it's better to save logs to ~/.cache/refact/logs/ by removing --logs-stderr.

The process does not restart with relevant settings change, specifically api_key, address_url, telemetry_code_snippets.

Errors that are kind of caught there:

			print(line)
			if "error" in line:
				self.statusbar.update_statusbar("error", line)

-- but they really need to go via LSP RPC. The simple test is to turn off wi-fi, does this produce a disconnected icon in the status bar or not?

@digital-phoenix
Copy link
Contributor

I just confirmed that turning off the wi-fi produces a timeout error in the status bar.

@olegklimov
Copy link
Contributor Author

produces a timeout error in the status bar

Cool, but we need error to go via LSP, not through stderr. We want a good product, written correctly, no hacks.

@digital-phoenix
Copy link
Contributor

The error is generated from the rpc connection. The rpc connection uses stdio in favour of an http connection because the server is running locally.

@digital-phoenix
Copy link
Contributor

The main reason the code monitors the process through stderr is to address the issue where the server would crash but the process would not terminate. By monitoring for crashes directly the code is able to cleanly close the server.

@olegklimov
Copy link
Contributor Author

refact-lsp does not crash. I think we fixed all or nearly all error handling in rust, so unlikely it will panic or exit.

@digital-phoenix
Copy link
Contributor

This code is over a month old. If the refact server doesn't crash anymore then this code can be removed.

@olegklimov
Copy link
Contributor Author

image

I just exited Sublime Text, two processes left over.

@olegklimov
Copy link
Contributor Author

Tested: process does not restart

The process does not restart with relevant settings change, specifically api_key, address_url, telemetry_code_snippets.

^^ this specifically doesn't work.

image

Here if I change "zzz" to "www" it doesn't restart the refact-lsp process with the new command line.

@hazratisulton
Copy link

Checked #16 - changing option in plugin settings restarts the refact-lsp.

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

No branches or pull requests

3 participants