Skip to content

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jan 21, 2024

Fixes #8368
Fixes #9186

Summary

Arbitrary TOML strings can be provided via the command-line to override configuration options in pyproject.toml or ruff.toml. As an example: to run over typeshed and respect typeshed's pyproject.toml, but override a specific isort setting and enable an additional pep8-naming setting:

cargo run -- check ../typeshed --no-cache --config ../typeshed/pyproject.toml --config "lint.isort.combine-as-imports=false" --config "lint.extend-select=['N801']"

This is what the error message currently looks like if you provide an invalid --config option:

>cargo run -- check ../typeshed --no-cache --config foo.toml
    Finished dev [unoptimized + debuginfo] target(s) in 0.77s
     Running `target\debug\ruff.exe check ../typeshed --no-cache --config foo.toml`
error: invalid value 'foo.toml' for '--config <CONFIG_OPTION>'

  tip: The `--config` flag must either be a path to a `.toml` configuation file or a TOML string providing configuration overrides
  tip: The path `foo.toml` does not exist on your filesystem

For more information, try '--help'.
error: process didn't exit successfully: `target\debug\ruff.exe check ../typeshed --no-cache --config foo.toml` (exit code: 2)

Test Plan

So far, just manual testing. TODO: write proper tests and docs.

Several tests added to crates/ruff/tests/format.rs and crates/ruff/tests/lint.rs. Also, extensive manual testing.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Nice! I haven't done a full review and only skimmed through the code.

Some CLI tests showing how overrides work for check and format, including some advanced use cases like specifying values for a table (e.g is there a way to "append" to --per-file-ignores without having to use a JSON value?) Can I do --config "per-file-ignores.__init__.py" = ["RUF100"]? See ruff/tests/format or /ruff/tests/lint etc for examples.

Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Nice work!

BurntSushi added a commit that referenced this pull request Jan 24, 2024
Previously, without the 'wrap_help' feature enabled, Clap would not do
any auto-wrapping of help text. For help text with long lines, this
tends to lead to non-ideal formatting. It can be especially difficult to
read when the width of the terminal is smaller.

This commit enables 'wrap_help', which will automatically cause Clap to
query the terminal size and wrap according to that. Or, if the terminal
size cannot be determined, it will default to a maximum line width of
100.

Ref #9599 (comment)
BurntSushi added a commit that referenced this pull request Jan 24, 2024
Previously, without the 'wrap_help' feature enabled, Clap would not do
any auto-wrapping of help text. For help text with long lines, this
tends to lead to non-ideal formatting. It can be especially difficult to
read when the width of the terminal is smaller.

This commit enables 'wrap_help', which will automatically cause Clap to
query the terminal size and wrap according to that. Or, if the terminal
size cannot be determined, it will default to a maximum line width of
100.

Ref #9599 (comment)
BurntSushi added a commit that referenced this pull request Jan 24, 2024
Previously, without the 'wrap_help' feature enabled, Clap would not do
any auto-wrapping of help text. For help text with long lines, this
tends to lead to non-ideal formatting. It can be especially difficult to
read when the width of the terminal is smaller.

This commit enables 'wrap_help', which will automatically cause Clap to
query the terminal size and wrap according to that. Or, if the terminal
size cannot be determined, it will default to a maximum line width of
100.

Ref #9599 (comment)
BurntSushi added a commit that referenced this pull request Jan 24, 2024
Previously, without the 'wrap_help' feature enabled, Clap would not do
any auto-wrapping of help text. For help text with long lines, this
tends to lead to non-ideal formatting. It can be especially difficult to
read when the width of the terminal is smaller.

This commit enables 'wrap_help', which will automatically cause Clap to
query the terminal size and wrap according to that. Or, if the terminal
size cannot be determined, it will default to a maximum line width of
100.

Ref #9599 (comment)
@github-actions
Copy link
Contributor

github-actions bot commented Jan 24, 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.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 25, 2024

CodSpeed Performance Report

Merging #9599 will not alter performance

Comparing AlexWaygood:arbitrary-cli-overrides (0f8a583) with main (d387d0b)

Summary

✅ 30 untouched benchmarks

@AlexWaygood
Copy link
Member Author

Thanks so much @zanieb! I think I tackled nearly all of your points, except for #9599 (comment) where we're waiting for Charlie to chime in.

Some of the error messages might still be a bit verbose for your liking, but I'm not sure how to trim them down... suggestions welcome :)

@charliermarsh
Copy link
Member

Sorry, didn't realize this was waiting on me! I'll take a look today, then we should merge :)

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Looks great overall.


// small hack so that multiline tips
// have the same indent on the left-hand side:
let tip_indent = " ".repeat(" tip: ".len());
Copy link
Member

Choose a reason for hiding this comment

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

A little concerning how much implicit coupling there is to Clap, but I don't have better suggestions (and at least there's test coverage, I think, such that if we upgrade Clap and the output format changes, we will know that there's a disconnect here).

Copy link
Member Author

@AlexWaygood AlexWaygood Feb 9, 2024

Choose a reason for hiding this comment

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

Yeah. Ideally I feel like clap would have a feature that allows long error messages to be wrapped to the terminal width, much as it does with the output of --help. But it doesn't seem to have that feature right now :/

Initially I had each sentence as a separate tip, which obviated the need for this kind of hack, but Zanie (correctly!) pointed out that that looked pretty weird

@AlexWaygood
Copy link
Member Author

Thanks all for the reviews! I'll land this now

@AlexWaygood AlexWaygood enabled auto-merge (squash) February 9, 2024 21:18
@AlexWaygood AlexWaygood disabled auto-merge February 9, 2024 21:20
@AlexWaygood AlexWaygood merged commit 8ec5627 into astral-sh:main Feb 9, 2024
@AlexWaygood AlexWaygood deleted the arbitrary-cli-overrides branch February 9, 2024 21:56
@ofek
Copy link
Contributor

ofek commented Feb 19, 2024

For posterity: #10035

charliermarsh added a commit that referenced this pull request Mar 4, 2024
## 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.
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
…stral-sh#9599)

Fixes astral-sh#8368
Fixes astral-sh#9186

## Summary

Arbitrary TOML strings can be provided via the command-line to override
configuration options in `pyproject.toml` or `ruff.toml`. As an example:
to run over typeshed and respect typeshed's `pyproject.toml`, but
override a specific isort setting and enable an additional pep8-naming
setting:

```
cargo run -- check ../typeshed --no-cache --config ../typeshed/pyproject.toml --config "lint.isort.combine-as-imports=false" --config "lint.extend-select=['N801']"
```

---------

Co-authored-by: Micha Reiser <[email protected]>
Co-authored-by: Zanie Blue <[email protected]>
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

cli Related to the command-line interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docstring code formatter: add a --docstring-code-format parameter to ruff format CLI Allow override of configuration options via the CLI

6 participants