python-lsp / python-lsp-ruff

Linter plugin for pylsp based on ruff.
MIT License
144 stars 20 forks source link

Should custom config override local pyproject.toml / ruff.toml? #91

Open iod-ine opened 1 month ago

iod-ine commented 1 month ago

Currently, when custom config is set during setup, project-local configuration files are ignored. I wonder whether this behavior is the result of an explicit design decision, or if it is open for discussion and subject to change. I imagined the setting to be a default config, used not to set the same settings across all projects, but to separate the configuration of LSP and Ruff and overridden by local settings.

jhossbach commented 1 month ago

python-lsp-ruff should not ignore project-local settings other than stuff like unsafe fixes, and extend-* settings. The idea is that any config settings like select are ignored if a project ruff config is present. You can still add additional linter settings for your editor (e.g. documentation linting "D" codes) that are not used by the project but you find good-to-have, these will not be overriden if a ruff config exists. Does that clear up things/make sense?

iod-ine commented 1 month ago

Yes, thank you :)

But more specifically, I am interested in the config setting. Here is the snippet of my Neovim config:

lspconfig.pylsp.setup({
    settings = {
        pylsp = {
            plugins = {
                ruff = {
                    enabled = true,
                    formatEnabled = true,
                    config = "~/.dotfiles/ruff.toml",
                }
            },
        },
    },
})

This makes any Python file use my standard Ruff config. But that also makes python-lsp-ruff ignore local ruff.toml, which I expected would take precedence over the default ~/.dotfiles/ruff.toml. Is that desired? Or should the presence of a local config file override not only select, exclude, and others, but also config?

jhossbach commented 1 month ago

Interesting, the desired behavior should be to ignore the config setting if a local ruff.toml is present. I don't see any reason why this should not be the case, can you add

lspconfig.pylsp.setup({
    settings = {
        pylsp = {
            plugins = {
                ruff = {
                    enabled = true,
+                   executable = {"<path-to-pylsp>", "-vvv", "--log-file", "/tmp/lsp.log"},
                    formatEnabled = true,
                    config = "~/.dotfiles/ruff.toml",
                }
            },
        },
    },
})

And then search for a line like "Found existing configuration for ruff, skipping pylsp config." in the logs? If it doesn't exist that means that your ruff.toml is not properly detected by pylsp.

iod-ine commented 1 month ago

Had to put it like this:

lspconfig.pylsp.setup({
    cmd = {"<path-to-pylsp>", "-vvv", "--log-file", "/tmp/lsp.log"},
    settings = {
        pylsp = {
            plugins = {
                ruff = {
                    enabled = true,
                    formatEnabled = true,
                    config = "~/.dotfiles/ruff.toml",
                }
            },
        },
    },
})

Found something interesting: a local ruff.toml does not override anything at all, even select. A local pyproject.toml however does everything as expected, overriding everything it should. At the same time, if nothing is configured in the setup call, a local ruff.toml works fine, but there is no "skipping pylsp config" line in the log.

jhossbach commented 1 month ago

Hmm, can you post the full log?

iod-ine commented 1 month ago

Here are all three possible states I tried: [REDACTED]

jhossbach commented 1 month ago

I took the liberty of removing the logs since your logs were showing the contents of the python file you were editing and I wasn't sure whether it contained sensitive information. The problem is that pylsp cannot find your project root directory. As an example, this does not find a project root:

.
├── ruff.toml
└── test_project
    ├── __init__.py
    └── test.py

Adding a pyproject.toml to the . directory will tell pylsp that this is the root directory from what I can tell, @ccordoba12 can you shed some light on how pylsp figures out the root used as workspace.root_path?

In short: This method does not work since workspace.root_path is None: https://github.com/python-lsp/python-lsp-ruff/blob/ba3c25fbc87fc17debfe23e04b7da4e982701d9e/pylsp_ruff/plugin.py#L718-L720

iod-ine commented 1 month ago

Thank you! I made sure to remove anything sensitive, just wanted to test on a realistic file that made it easy to see what configuration is being applied.

Yes, just tested that adding a blank pyproject.toml indeed makes a difference and the config is loaded properly. Adding a .git directory has the same effect. I can use either of those as a workaround for my use cases where it matters.