koxudaxi / ruff-pycharm-plugin

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

use macros for `projectRuffExecutablePath` #381

Open KotlinIsland opened 4 months ago

KotlinIsland commented 4 months ago

The code in here works, but is really bad. I couldn't figure out how to make macros work properly.

Currently the plugin always saves the full path to the config file. This makes it impossible to commit project config. In this change we collapse the venv dir before it is saved.

koxudaxi commented 4 months ago

Thank you for creating the PR. I will review it tonight.

koxudaxi commented 4 months ago

@KotlinIsland

I would recommend against using $PROJECT_DIR$ and instead use $PyInterpreterDirectory$, it would be highly unlikely that a project would include an exe for ruff outside the venv, and some venvs are not within the project dir (poetry)

Sorry for the delay. I checked the operation. I agree with you. Please let me test it some more.

koxudaxi commented 3 months ago

@KotlinIsland I'm sorry for my late reply. I tested the your PR in my windows OS. image

The ruff binary file name includes the .exe suffix. Do you expect the behavior? Should you use the special variable for ruff binary, like $ruff$ to inject the actual binary file name for each OS?

KotlinIsland commented 3 months ago

Ahh, good point, I didn't anticipate this complication. I see two possibilities:

  1. We point to the directory containing the executable, instead of the executable itself.
  2. We automatically handle adding/removing the .exe as needed.
koxudaxi commented 3 months ago

We point to the directory containing the executable, instead of the executable itself.

It is certainly easier to save to file this way. However, for clarity regarding the display on the settings screen, it might be better to include up to .exe.

KotlinIsland commented 3 months ago

for clarity

I completely agree. Additionally, this PR contains only a rough draft, I couldn't figure out how to make macro fields work properly.

koxudaxi commented 3 months ago

Additionally, this PR contains only a rough draft, I couldn't figure out how to make macro fields work properly. OK, I will check it today.

koxudaxi commented 3 months ago

@KotlinIsland

I did some more research. Here is a code that might be helpful. https://github.com/JetBrains/intellij-community/blob/94282b7d391a5ed702f01a31549b4893c62974f1/python/src/com/jetbrains/python/sdk/PySdkSettings.kt#L52-L81

I think that before writing the Path to the config file, we can set the macro as inject and expand it again if we want to refer to it. However, I think there needs to be some caching mechanism to avoid the overhead of executing the code here every time it is referenced.

koxudaxi commented 3 months ago

@KotlinIsland I thought about it some more, but I felt that if We don't need to use a macro to select the bin of the project, if ruff plugin gets the ruff or ruff.exe. In other words, what if the user can choose to either use the project's venv bin or a custom executable? I think the custom executables should not be shared with another developer in git.

koxudaxi commented 3 months ago

@KotlinIsland What do you think?

KotlinIsland commented 3 months ago

Looks great!