Skip to content

Conversation

@InSyncWithFoo
Copy link
Contributor

Summary

Resolves #13995.

Test Plan

cargo nextest run and cargo insta test.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood self-requested a review January 5, 2025 11:15
@AlexWaygood
Copy link
Member

AlexWaygood commented Jan 5, 2025

This is great! I think we can do even better with a few tweaks, though. With your branch currently, this is the error message I get:

~/dev/ruff (cli-config)⚡ % cargo run -- check . --config='lint.flake8-pytest-style="csv"'
   Compiling ruff v0.8.6 (/Users/alexw/dev/ruff/crates/ruff)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.51s
     Running `target/debug/ruff check . '--config=lint.flake8-pytest-style="csv"'`
error: invalid value 'lint.flake8-pytest-style="csv"' for '--config <CONFIG_OPTION>'

  tip: A `--config` flag must either be a path to a `.toml` configuration file
       or a TOML `<KEY> = <VALUE>` pair overriding a specific configuration
       option

`lint.flake8-pytest-style` is a table.
Did you mean to use one of its subkeys instead? Possible choices:
lint.flake8-pytest-style.fixture-parentheses
lint.flake8-pytest-style.parametrize-names-type
lint.flake8-pytest-style.parametrize-values-type
lint.flake8-pytest-style.parametrize-values-row-type
lint.flake8-pytest-style.raises-require-match-for
lint.flake8-pytest-style.raises-extend-require-match-for
lint.flake8-pytest-style.mark-parentheses

Could not parse the supplied argument as a `ruff.toml` configuration option:

invalid type: string "csv", expected struct Flake8PytestStyleOptions
in `lint`

For more information, try '--help'.
Colorized screenshot

image

But if I apply this diff to your branch:

--- a/crates/ruff/src/args.rs
+++ b/crates/ruff/src/args.rs
@@ -978,23 +978,25 @@ The path `{value}` does not point to a configuration file"
                     let prefixed_subkeys = format!("{set}")
                         .trim_ascii()
                         .split('\n')
-                        .map(|child| format!("{key}.{child}"))
+                        .map(|child| format!(" - `{key}.{child}`"))
                         .join("\n");
 
                     tip.push_str(&format!(
                         "
 
-`{key}` is a table.
-Did you mean to use one of its subkeys instead? Possible choices:
+`{key}` is a table of configuration options, but only a single option can be
+overridden with `--config`. Did you want to override one of the table's subkeys?
+
+Possible choices:
 {prefixed_subkeys}"
                     ));
                 }
-            };
-
-            tip.push_str(&format!(
-                "\n\n{}:\n\n{underlying_error}",
-                config_parse_error.description()
-            ));
+            } else {
+                tip.push_str(&format!(
+                    "\n\n{}:\n\n{underlying_error}",
+                    config_parse_error.description()
+                ));
+            }
         }

Then it becomes:

~/dev/ruff (cli-config)⚡ [2] % cargo run -- check . --config='lint.flake8-pytest-style="csv"'
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.36s
     Running `target/debug/ruff check . '--config=lint.flake8-pytest-style="csv"'`
error: invalid value 'lint.flake8-pytest-style="csv"' for '--config <CONFIG_OPTION>'

  tip: A `--config` flag must either be a path to a `.toml` configuration file
       or a TOML `<KEY> = <VALUE>` pair overriding a specific configuration
       option

`lint.flake8-pytest-style` is a table of configuration options, but only a single option can be
overridden with `--config`. Did you want to override one of the table's subkeys?

Possible choices:
 - `lint.flake8-pytest-style.fixture-parentheses`
 - `lint.flake8-pytest-style.parametrize-names-type`
 - `lint.flake8-pytest-style.parametrize-values-type`
 - `lint.flake8-pytest-style.parametrize-values-row-type`
 - `lint.flake8-pytest-style.raises-require-match-for`
 - `lint.flake8-pytest-style.raises-extend-require-match-for`
 - `lint.flake8-pytest-style.mark-parentheses`

For more information, try '--help'.
Colorized screenshot

image

which feels significantly more readable to me. WDYT?

@InSyncWithFoo
Copy link
Contributor Author

InSyncWithFoo commented Jan 5, 2025

@AlexWaygood I was actually aiming for copyability, but readability is fine too.

[...] but only a single option can be overridden with --config.

This is rather misleading, because you can pass an inline table as the value:

# Same as "lint.select = ['I001']"
$ ruff check --isolated --config="lint = { select = ['I001'] }"

@AlexWaygood
Copy link
Member

This is rather misleading, because you can pass an inline table as the value:

Oh, great point! I forgot how my own feature worked :-) We can scrap that clause, then.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@MichaReiser MichaReiser added the cli Related to the command-line interface label Jan 6, 2025
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.

This looks great. I'm a bit worried about the output parsing and I think we can improve the handling if set itself has sub-table entries.

@MichaReiser MichaReiser enabled auto-merge (squash) January 6, 2025 13:11
@AlexWaygood AlexWaygood disabled auto-merge January 6, 2025 13:14
@AlexWaygood AlexWaygood enabled auto-merge (squash) January 6, 2025 13:15
@AlexWaygood AlexWaygood merged commit 832c0fa into astral-sh:main Jan 6, 2025
20 checks passed
@InSyncWithFoo InSyncWithFoo deleted the cli-config branch January 6, 2025 13:23
dcreager added a commit that referenced this pull request Jan 6, 2025
* main: (60 commits)
  [`ruff`] Dataclass enums (`RUF049`) (#15299)
  Better error message when `--config` is given a table key and a non-inline-table value (#15266)
  Update pre-commit dependencies (#15289)
  Don't fix in ecosystem check (#15267)
  Update Rust crate itertools to 0.14.0 (#15287)
  Remove accidental empty block at the bottom of `split-static-string (SIM905)` doc (#15290)
  Update Rust crate clearscreen to v4 (#15288)
  Update Rust crate insta to v1.42.0 (#15286)
  Update NPM Development dependencies (#15285)
  Update dependency uuid to v11.0.4 (#15284)
  Update dependency ruff to v0.8.6 (#15283)
  Update Rust crate syn to v2.0.95 (#15282)
  Update Rust crate matchit to v0.8.6 (#15281)
  Update Rust crate bstr to v1.11.3 (#15280)
  [red-knot] Future-proof `Type::is_disjoint_from()` (#15262)
  [red-knot] Improve `Type::is_disjoint_from()` for `KnownInstanceType`s (#15261)
  [red-knot] Minor simplifications and improvements to constraint narrowing logic (#15270)
  Allow assigning ellipsis literal as parameter default value (#14982)
  [red-knot] fix control flow for assignment expressions in elif tests (#15274)
  [`refurb`] Mark fix as unsafe when the right-hand side is a string (`FURB171`) (#15273)
  ...
dcreager added a commit that referenced this pull request Jan 6, 2025
* main: (29 commits)
  [`ruff`] Dataclass enums (`RUF049`) (#15299)
  Better error message when `--config` is given a table key and a non-inline-table value (#15266)
  Update pre-commit dependencies (#15289)
  Don't fix in ecosystem check (#15267)
  Update Rust crate itertools to 0.14.0 (#15287)
  Remove accidental empty block at the bottom of `split-static-string (SIM905)` doc (#15290)
  Update Rust crate clearscreen to v4 (#15288)
  Update Rust crate insta to v1.42.0 (#15286)
  Update NPM Development dependencies (#15285)
  Update dependency uuid to v11.0.4 (#15284)
  Update dependency ruff to v0.8.6 (#15283)
  Update Rust crate syn to v2.0.95 (#15282)
  Update Rust crate matchit to v0.8.6 (#15281)
  Update Rust crate bstr to v1.11.3 (#15280)
  [red-knot] Future-proof `Type::is_disjoint_from()` (#15262)
  [red-knot] Improve `Type::is_disjoint_from()` for `KnownInstanceType`s (#15261)
  [red-knot] Minor simplifications and improvements to constraint narrowing logic (#15270)
  Allow assigning ellipsis literal as parameter default value (#14982)
  [red-knot] fix control flow for assignment expressions in elif tests (#15274)
  [`refurb`] Mark fix as unsafe when the right-hand side is a string (`FURB171`) (#15273)
  ...
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.

Cannot set config option with struct type from command-line

3 participants