Skip to content
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

Add a new configurationPreference value filesystemOnly #16209

Open
dhruvmanila opened this issue Feb 17, 2025 · 9 comments · May be fixed by #16258
Open

Add a new configurationPreference value filesystemOnly #16209

dhruvmanila opened this issue Feb 17, 2025 · 9 comments · May be fixed by #16258
Labels
needs-decision Awaiting a decision from a maintainer server Related to the LSP server

Comments

@dhruvmanila
Copy link
Member

Description

Thanks for implementing the ruff.configurationPreference setting. I wanted to raise one behavior that I found confusing:

If some ruff settings are not set in the .toml file, the extension will use preferences from the editor, not the default values. I assume this is what "takes priority" mean?

This results in undesirable behaviors, including inconsistencies in linting and formatting. For example, if line length is not defined in the .toml file but defined in settings.json, running ruff format in the terminal will apply the default of 88 while vscode's format will apply the settings.json length.

Maybe there is an easy workaround I missed? If not, would it be possible to have a "filesystemOnly" option that would use the editor's preferences only when no .toml is present? Alternatively, could we have "hard" vs "soft" priorities for both "editorFirst" and "filesystemFirst"?

Thanks in advance

Originally posted by @VictorGillioz in #425

@dhruvmanila
Copy link
Member Author

We'd need to be careful around versioning as this would be added in Ruff and the VS Code extension but if there's a mismatch between the version the extension shouldn't error.

@InSyncWithFoo InSyncWithFoo linked a pull request Feb 19, 2025 that will close this issue
@T-256
Copy link
Contributor

T-256 commented Feb 19, 2025

This results in undesirable behaviors, including inconsistencies in linting and formatting. For example, if line length is not defined in the .toml file but defined in settings.json, running ruff format in the terminal will apply the default of 88 while vscode's format will apply the settings.json length.

Maybe there is an easy workaround I missed?

I think it is editor-related feature and already could be handled by VSCode: reset modified settings per workspace/folder.

Also, IMO, ruff.configurationPreference setting could have only two varients: editor and filesystem. Use separated settings fileystemDiscover with default of true and set to false for apply editorOnly behavior.

Since the preference word points to selecting one among others, I think it makes more sense to NOT having First suffix in its varients.

@dhruvmanila
Copy link
Member Author

I think it is editor-related feature and already could be handled by VSCode: reset modified settings per workspace/folder.

Can you say more about this? What's "VSCode: reset modified settings"?

I think the user still wants to keep the settings but not use it, it does seem counterintuitive to still keep them.

@dhruvmanila dhruvmanila added needs-decision Awaiting a decision from a maintainer and removed needs-decision Awaiting a decision from a maintainer labels Feb 20, 2025
@MichaReiser
Copy link
Member

Reading the issue description. I don't think filesystemOnly would do what I'd expect. I'd expect that filesystemOnly would disregard the editor settings completely even if no configuration exists for the current project. But that's not what's proposed. The proposed behavior is to use the file system configuration if it exists, then fall back to the editor one (but never merge the configurations).

I do agree with @T-256 that I don't think a filesystemOnly as I understood makes sense. In that case, users should unset their editor settings.

Maybe it's just a naming issue, but I get the impression that configuration precedence is trying to do too much.

  1. It specifies in which precedence options from the discovered configurations should be merged
  2. It configures which configurations should be discovered/respected

That makes me wonder if we should have a dedicated settings instead.

A related option -- that I'm surprised it has never come up before -- is whether Ruff should run on projects that don't have a project configuration. I don't think we have to solve this here, but maybe something to keep in mind.

@InSyncWithFoo
Copy link
Contributor

@MichaReiser Currently editorOnly means "discard any configurations found in .toml files". filesystemOnly, if added, must work similarly to preserve consistency. These two have quite unfortunate names, sure, but the ship has sailed.

Instead, a fifth option with the meaning "use .toml files if found, fall back to editor otherwise" should be added (tentatively named filesystemFallingBackToEditor).

@MichaReiser
Copy link
Member

These two have quite unfortunate names, sure, but the ship has sailed.

I still see an opportunity to error correct. We have to support editorOnly for the near future.

Either way, I don't think a filesystemOnly setting makes sense. Users should unset their editor settings in that case.

@dhruvmanila dhruvmanila added the needs-decision Awaiting a decision from a maintainer label Feb 20, 2025
@dhruvmanila
Copy link
Member Author

(I'm going to mark this as "needs-decision" as after talking with Micha I agree that we can improve here.)

@T-256
Copy link
Contributor

T-256 commented Feb 20, 2025

I think the user still wants to keep the settings but not use it, it does seem counterintuitive to still keep them.

You can keep them as User settings and overriding them to default values in Workspace settings.
Your workflow should become like these:

  • User Settings:
    {
      "ruff.configurationPreference": "filesystemFirst",
      "ruff.lint.extendSelect": [
          "RUF013",  // implicit-optional (RUF013)
      ],
    }
  • Workspace Settings - Where you want apply configs from FileSystem only and all editor settings to be ignored:
    {
      // seetings in this file would be merge to `User Settings`
      "ruff.lint.extendSelect": [],  // reset modified settings of `User Settings`,
                                     // So, now it seems there is no configuration set on editor side for ruff server.
    }

Then only filesystem configs are respected.

@dhruvmanila
Copy link
Member Author

You can keep them as User settings and overriding them to default values in Workspace settings.

That's a good suggestion, thanks. @VictorGillioz does this help your use-case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer server Related to the LSP server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants