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

Move shell expansion into --config lookup #10219

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Move shell expansion into --config lookup #10219

merged 2 commits into from
Mar 4, 2024

Conversation

charliermarsh
Copy link
Member

Summary

When users provide configurations via --config, we use shellexpand to ensure that we expand signifiers like ~ and environment variables.

In #9599, we modified --config to accept either a path or an arbitrary setting. However, the detection (to determine whether the value is a path or a setting) was lacking the shellexpand behavior -- it was downstream. So we were always treating paths like ~/ruff.toml as values, not paths.

Closes astral-sh/ruff-vscode#413.

@charliermarsh charliermarsh added bug Something isn't working configuration Related to settings and configuration labels Mar 4, 2024
@charliermarsh charliermarsh force-pushed the charlie/path branch 6 times, most recently from 0c765a3 to 9212c42 Compare March 4, 2024 01:55
Copy link
Contributor

github-actions bot commented Mar 4, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -8 violations, +0 -0 fixes in 1 projects; 42 projects unchanged)

pandas-dev/pandas (+0 -8 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- pandas/core/internals/__init__.py:21:5: PLC0415 `import` should be at the top-level of a file
- pandas/core/internals/__init__.py:33:9: PLC0415 `import` should be at the top-level of a file
- pandas/core/internals/__init__.py:37:16: PLR6201 Use a `set` literal when testing for membership
- pandas/core/internals/__init__.py:53:13: PLC0415 `import` should be at the top-level of a file
- pandas/core/internals/__init__.py:57:13: PLC0415 `import` should be at the top-level of a file
- pandas/core/internals/__init__.py:61:13: PLC0415 `import` should be at the top-level of a file
- pandas/core/internals/__init__.py:65:13: PLC0415 `import` should be at the top-level of a file
- pandas/core/internals/__init__.py:69:13: PLC0415 `import` should be at the top-level of a file

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
PLC0415 7 0 7 0 0
PLR6201 1 0 1 0 0

crates/ruff/src/args.rs Outdated Show resolved Hide resolved
crates/ruff/src/args.rs Show resolved Hide resolved
@MichaReiser MichaReiser added cli Related to the command-line interface and removed configuration Related to settings and configuration labels Mar 4, 2024
crates/ruff/tests/lint.rs Outdated Show resolved Hide resolved
crates/ruff/src/args.rs Show resolved Hide resolved
@AlexWaygood
Copy link
Member

Thanks for fixing -- sorry I missed this!

@charliermarsh charliermarsh enabled auto-merge (squash) March 4, 2024 17:23
}
// Convert to UTF-8.
let Some(value) = value.to_str() else {
// But respect non-UTF-8 paths.
Copy link
Member

Choose a reason for hiding this comment

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

Worth adding a test with a non-UTF-8 path?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm gonna defer it, I don't even know if Ruff as a whole will work with non-UTF-8 paths.

@charliermarsh charliermarsh disabled auto-merge March 4, 2024 17:29
@charliermarsh charliermarsh merged commit 7eaec30 into main Mar 4, 2024
17 checks passed
@charliermarsh charliermarsh deleted the charlie/path branch March 4, 2024 17:45
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
## Summary

When users provide configurations via `--config`, we use `shellexpand`
to ensure that we expand signifiers like `~` and environment variables.

In astral-sh#9599, we modified `--config`
to accept either a path or an arbitrary setting. However, the detection
(to determine whether the value is a path or a setting) was lacking the
`shellexpand` behavior -- it was downstream. So we were always treating
paths like `~/ruff.toml` as values, not paths.

Closes astral-sh/ruff-vscode#413.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Related to the command-line interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail to recognize the Ruff config file path in the VSCode settings.json
3 participants