koxudaxi / ruff-pycharm-plugin

PyCharm plugin for ruff. This plugin provides reformat code using ruff.
MIT License
196 stars 9 forks source link

Failed Ruff runs should be displayed as errors, not raised as IDE exception #294

Open lensvol opened 10 months ago

lensvol commented 10 months ago

Describe the bug When the plugin calls ruff binary and it exits with an unexpected error (e.g. there is a typo in ruff.toml), the user will not get notified about that, but instead, an IDE exception is being raised. Since those are not easy to find (small red icon in the right corner) and require you to parse Kotlin traceback message with your eyes it is difficult to debug the exact cause of an error.

To Reproduce Steps to reproduce the behavior:

  1. Create sample ruff.toml with the following contents:
    python = "3.12" 
  2. In the plugin settings choose this file as a chosen Ruff configuration.
  3. Make some changes to an open Python file.
  4. Save the file.

Expected behavior A pop-up that relays the exact error message produced by failed ruff run.

Environments (please complete the following information):

If you agree that this is a valid issue but don't have time to work on it, I can make a PR for that.

koxudaxi commented 10 months ago

@lensvol Thank you for creating the issue and sponsoring me 🙇

If you agree that this is a valid issue but don't have time to work on it, I can make a PR for that.

I agree with your observation. It would be great if you could create a PR to address this. However, before that, I have one concern. How would the plugin behave if the user is in the middle of editing the toml file and ruff gets executed, resulting in an error? Would it not become a nuisance alert for the user?

lensvol commented 10 months ago

I do not see how that specific scenario is possible when editing TOML in the IDE, honestly. As far as I can see, Ruff binary will only get called in the following situations:

  1. Explicit call via Run Ruff
  2. When the current file is going to be saved.
  3. When code is being reformatted.

Cases 2 and 3 have safeguards in the form of isApplicableTo, which will prevent Ruff execution due to the file not being a Python one. Case 1 probably will not annoy the user, since the appearance of that pop-up is caused by their own hands.

Speaking more broadly, Ruff execution errors could also be reported via notification balloon instead of a modal pop-up.

koxudaxi commented 10 months ago

@lensvol Sorry, I did not consider that isApplicableTo. Please ignore my concern.

Speaking more broadly, Ruff execution errors could also be reported via notification balloon instead of a modal pop-up.

I prefer a notificaiton ballon :)

KotlinIsland commented 6 months ago

How would the plugin behave if the user is in the middle of editing the toml file and ruff gets executed, resulting in an error? Would it not become a nuisance alert for the user?

This would actually be highly based if the plugin could run on pyproject.toml

[tool.ruff]
target-version = "py312"
asdf = true
> ruff check
ruff failed
  Cause: Failed to parse C:\Users\AMONGUS\projects\test-python\pyproject.toml
  Cause: TOML parse error at line 521, column 1
    |
521 | [tool.ruff]
    | ^^^^^^^^^^^
unknown field `asdf`

So we could show the error in the toml file:

[tool.ruff] # ruff failed: unknown field `asdf`
target-version = "py312"
asdf = true
KotlinIsland commented 6 months ago

Could I suggest a file level banner error over a balloon notification, I fell it would be better suited.