fall back to system ruff executable without executable setting

[TheJJ]

otherwise I have to either configure the path in the project's pyproject.toml or unnecessarily in the lsp configuration :)

[jhossbach]

Hi, sorry for the delay. I am not sure what the use case for this is though - if the executable is not set the ruff should automatically find the "correct" executable by using python -m ruff, see here:

python-lsp-ruff/pylsp_ruff/plugin.py

Lines 534 to 541 in 9117709

	if p is None:
	log.debug(
	f"Calling ruff via '{sys.executable} -m ruff'"
	f" with args: {arguments} on '{document_path}'"
	)
	cmd = [sys.executable, "-m", "ruff", str(subcommand)]
	cmd.extend(arguments)
	p = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)

Does this not work for you?

[TheJJ]

Hi! - no worries :)
Yes, python3 -m ruff does not work for me, since the Gentoo dev-util/ruff package doesn't install the Python module
Apart from shell completions and documentation, I only have /usr/bin/ruff.

I think it's alright to use shutil.which to search in the PATH for the executable, since I don't see a useful way to hard-code the executable path in the in-project pyproject.toml, and hard-coding it in the lsp configuration doesn't seem right when it's just available in the PATH.

What do you think?

[jhossbach]

Ah, I wasn't aware of this issue.
I would suggest including os.shutil in case python -m ruff fails, since I don't know if removing that could potentially affect others.
I'll add something to the PR.

[TheJJ]

you could have a look once if ruff has an entry point in from importlib.metadata import entry_points, so we don't have to check it again and again and execute a new python subprocess?

[jhossbach]

Instead of using importlib, I moved finding the correct executable to a new function which is now cached, so it should only run once with the given executable.
Can you try to see if that works?

[jhossbach]

Hmm, I seemed to have missed those issues before pushing, let me look into it.

[jhossbach]

This should work now, please check if it does.

[TheJJ]

from what I understand now, what you whish is:

• if executable is set and exists, use it
• otherwise, try the ruff module
• otherwise, use the system executable

I'd propose to implement is this way:

• if executable is set, use it, and crash if it doesn't exist. silent swiches of the used program are unexpected and hard to debug.
• otherwise, check once if the ruff module exists and use it
• otherwise, use the system executable found by which.

maybe some way to prefer the executable would be better, because then we could bypass invoking a new python interpreter for every ruff invocation eats a bit of latency. or calling the python module in the same interpreter.

[TheJJ]

I've implemented an improved version, which uses the configuration value without fallback, or it tries the python module and then system executable next.

one could optimize this to eliminate the extra python interpreter invocation if using the module (for latency improvement) by re-using the current interpreter instead.

[jhossbach]

Looks really nice, but I am not sure the ImportError is propagated the way you probably want it to, e.g. when testing with neovim the error does not reach the editor.
I think there is a workspace.show_message method which could work here (although I haven't tested it).

[TheJJ]

which import error, if no executable/module is found?

if there's a better way than crashing with an exception, we should surely use it :)

[jhossbach]

Similarly to log.error(...), the RunTimeError will only be seen in pylsps log file.
I just tried to try except the Error for a dummy executable and report it to the editor with workspace.show_message(...) but so far this did not work for me.
Unfortunately I don't have the time to look into it until next week.

[TheJJ]

maybe we could merge this for now since it's not making it worse than before when no ruff was found :)
then come up with sane error propagation to notify the user properly.

[jhossbach]

I took the liberty of removing the RuntimeErrors for now, otherwise thanks a lot for the PR!

[TheJJ]

great :)
thanks for the tool!