Skip to content

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

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

Open
iod-ine opened this issue May 16, 2024 · 17 comments
Open

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

iod-ine opened this issue May 16, 2024 · 17 comments

Comments

@iod-ine
Copy link

iod-ine commented May 16, 2024

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
Copy link
Member

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
Copy link
Author

iod-ine commented May 16, 2024

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
Copy link
Member

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
Copy link
Author

iod-ine commented May 17, 2024

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
Copy link
Member

Hmm, can you post the full log?

@iod-ine
Copy link
Author

iod-ine commented May 21, 2024

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

@jhossbach
Copy link
Member

jhossbach commented May 21, 2024

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:

ruff_toml = find_parents(
workspace.root_path, document_path, ["ruff.toml", ".ruff.toml"]
)

@iod-ine
Copy link
Author

iod-ine commented May 21, 2024

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.

@ccordoba12
Copy link
Member

Can you shed some light on how pylsp figures out the root used as workspace.root_path?

Sorry for the late reply. workspace.root_path is the path of the directory currently opened by your IDE or editor.

Some IDEs always force you to work with a project, and in that case its path is workspace.root_path.
However, most editors don't do that (e.g. VSCode, Vim, etc) and in that case workspace.root_path can be your home or None (not so sure about the last one though).

@minusf
Copy link

minusf commented Apr 3, 2025

hi. i am coming here to say the exact opposite 😆

For me the expected behaviour is that if i specify explicitly a config option for pylsp, I wish to override local autodetected pyproject.toml files.

My use case is to use more/stricter linting checks than my team mates that were agreed as a "linting baseline". (And this used to work until recently, my explicit config file was honoured)

I understand this cuts both ways (I could be using less checks than the local pyproject.toml file), however with most CLI utilities explicit arguments normally override default arguments, and for me the autodetected pyproject.toml file in the project root path is kind of a default argument.

I guess I could just migrate my .config/ruff/pyproject.toml file settings into the plugin's settings to extend/override autodetected pyproject.toml files? Although that would make it work only in vim/neovim...

@jhossbach
Copy link
Member

So you have a personal ~/.config/ruff/pyproject.toml that you wish to extend the project-local settings with? ruff itself only respects the closest configuration file and ignores all others: https://docs.astral.sh/ruff/configuration/#config-file-discovery.
I also suspect that ruff will ignore the project-local settings when pylsp.plugins.ruff.config is set to the path to your personal pyproject.toml.
I think the optimal way would be to configure the lsp options, or to adapt the project-local pyproject file without pushing to the remote

@minusf
Copy link

minusf commented Apr 3, 2025

The documentation says:

  1. If a configuration file is passed directly via --config, those settings are used for all analyzed files

This is why I had this expectation, as I think having pylsp.plugins.ruff.config is analogous to --config, and I think the "Principle of least astonishment" is somewhat violated here, esp if it's possible to override all other options using pylsp.plugins.ruff, except config.

Perhaps the comment in the configuration example could be more explicit:

      config = "<path_to_custom_ruff_toml>",  -- Custom config for ruff to use, unless a closer config file exists

or something similar...

@minusf
Copy link

minusf commented Apr 3, 2025

I also suspect that ruff will ignore the project-local settings when pylsp.plugins.ruff.config is set to the path to your personal pyproject.toml.

This is not what I see at the moment. I see in the debug log:

Found existing configuration for ruff, skipping pylsp config.

python_lsp_ruff-2.2.2

Even the comment in the code says:

# Check if pyproject is present, ignore user settings if toml exists

@minusf
Copy link

minusf commented Apr 3, 2025

let's look at the full story finally 😄

    # Check if pyproject is present, ignore user settings if toml exists
    if config_in_pyproject or ruff_toml:
        log.debug("Found existing configuration for ruff, skipping pylsp config.")
        # Leave config to pyproject.toml
        return PluginSettings(
            enabled=plugin_settings.enabled,
            format_enabled=plugin_settings.format_enabled,
            executable=plugin_settings.executable,
            unsafe_fixes=plugin_settings.unsafe_fixes,
            extend_ignore=plugin_settings.extend_ignore,
            extend_select=plugin_settings.extend_select,
            format=plugin_settings.format,
            severities=plugin_settings.severities,
        )

So it's a mix of both plugin settings, and the closest file... So my uneducated guess was incorrect:

if it's possible to override all other options using pylsp.plugins.ruff, except config.

However my suggestion to update the comment in the Lua config example is still valid...

Without any changes to the plugin, easiest solution for me is to define

            extend_ignore=plugin_settings.extend_ignore,
            extend_select=plugin_settings.extend_select,

in addition to the closest config file.

@minusf
Copy link

minusf commented Apr 3, 2025

Actually just moving config in the

      -- Rules that are ignored when a pyproject.toml or ruff.toml is present:

would solve this incorrect expectation already.

@jhossbach
Copy link
Member

That is not up to pylsp-ruff but a design choice of ruff itself. The config option is passed to ruff directly: https://github.com/python-lsp/python-lsp-ruff/blob/main/pylsp_ruff%2Fplugin.py#L613

@minusf
Copy link

minusf commented Apr 7, 2025

thank you for clarifying the config file argument. i probably made some mistake somewhere if my changes were not showing up in that explicitly passed argument.

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

4 participants