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

Read user configuration from ~/.config/ruff/ruff.toml on macOS #11115

Merged
merged 4 commits into from
Jun 24, 2024

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Apr 23, 2024

Summary

This PR moves Ruff's user-specific configuration from ~/Library/Application Support/ruff/ruff.toml to ~/.config/ruff/ruff.toml.

Many other tools do this. On my machine alone: dagger, zed, gatsby, gh, wandb, etc.

I also polled Twitter and it won in a landslide:

Screenshot 2024-04-23 at 4 07 19 PM

Let's ship this in v0.5.0, along with a deprecation warning (so we'll continue to respect ~/Library/Application Support/ruff/ruff.toml, but log a warning).

Closes #10739.

Test Plan

  • Created a file with an unused import.
  • Ran cargo check foo.py.
  • Verified that the deprecated configuration was loaded and respected, with a warning:
warning: Reading configuration from `~/Library/Application Support` is deprecated. Please move your configuration to `/Users/crmarsh/.config/ruff/ruff/ruff.toml`.
[2024-04-23][16:09:10][ruff::resolve][DEBUG] Using configuration file (via cwd) at: /Users/crmarsh/Library/Application Support/ruff/ruff.toml
  • Created ~/.config/ruff/ruff.toml.
  • Verified that the new configuration was loaded:
[2024-04-23][16:10:23][ruff::resolve][DEBUG] Using configuration file (via cwd) at: /Users/crmarsh/.config/ruff/ruff.toml

@charliermarsh charliermarsh added this to the v0.5.0 milestone Apr 23, 2024
@charliermarsh charliermarsh added breaking Breaking API change configuration Related to settings and configuration and removed breaking Breaking API change labels Apr 23, 2024
@charliermarsh charliermarsh requested review from zanieb and konstin April 23, 2024 20:11
@@ -34,6 +34,7 @@ crossbeam = { version = "0.8.4" }
dirs = { version = "5.0.0" }
drop_bomb = { version = "0.1.5" }
env_logger = { version = "0.11.0" }
etcetera = { version = "0.8.0" }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konstin suggested this crate, and it does what we want across Windows and Linux / macOS, but open to other suggestions.

Cargo.toml Outdated
@@ -34,6 +34,7 @@ crossbeam = { version = "0.8.4" }
dirs = { version = "5.0.0" }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove dirs in a future release, but it stays around for now for backwards compatibility.

Copy link
Contributor

github-actions bot commented Apr 23, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser
Copy link
Member

MichaReiser commented Apr 24, 2024

Sooo, we're doing twitter driven development now 😂

@konstin
Copy link
Member

konstin commented Apr 24, 2024

I would change the reasoning/description: Follow the XDG specification on all Unix platforms. Ruff will now use the XDG config directory to read user-level configuration (default: ~/.config/ruff/ruff.toml) over Library/Application Support/ruff/ruff.toml on macOS.

return Some(path);
// On macOS, we used to support reading from `/Users/Alice/Library/Application Support`.
if cfg!(target_os = "macos") {
let deprecated_config_dir = dirs::config_dir()?.join("ruff");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use https://docs.rs/etcetera/latest/etcetera/app_strategy/struct.Apple.html here and remove the dirs dependency (trading one dependency for another ;))

Copy link
Member

@MichaReiser MichaReiser Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I checked, we can use the Apple strategy but we need to use the data_dir to map to the Application Support folder (hardcoded).

The etcetera crate clearly documents that config_dir shouldn't be used on Mac when the config is user editable. I'm going to make this change.

From the dirs documentation:

// Mac: Some(/Users/Alice/Library/Application Support)
dirs::config_dir();

From the etcetera documentation:

assert_eq!(
    base_strategy.data_dir().strip_prefix(&home_dir),
    Ok(Path::new("Library/Application Support/"))
);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Feel free to merge.

@MichaReiser MichaReiser changed the base branch from main to ruff-0.5 June 24, 2024 12:26
@MichaReiser MichaReiser self-assigned this Jun 24, 2024
Copy link

codspeed-hq bot commented Jun 24, 2024

CodSpeed Performance Report

Merging #11115 will degrade performances by 4.69%

Comparing charlie/xdg (c4d143a) with ruff-0.5 (9348821)

Summary

❌ 1 regressions
✅ 29 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark ruff-0.5 charlie/xdg Change
linter/default-rules[pydantic/types.py] 1.8 ms 1.9 ms -4.69%

@MichaReiser
Copy link
Member

I ran through the test plan and verified the deprecation message.

@MichaReiser MichaReiser merged commit 25efb82 into ruff-0.5 Jun 24, 2024
19 of 20 checks passed
@MichaReiser MichaReiser deleted the charlie/xdg branch June 24, 2024 13:20
@MichaReiser MichaReiser added the breaking Breaking API change label Jun 24, 2024
@MichaReiser MichaReiser mentioned this pull request Jun 26, 2024
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
)

Co-authored-by: Micha Reiser <micha@reiser.io>
Closes #10739.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
)

Co-authored-by: Micha Reiser <micha@reiser.io>
Closes #10739.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking API change configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Global Config Discovery: use $XDG_CONFIG_HOME on macOS
3 participants