-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ruff
] Add analyze.string-imports-min-dots
to settings
#20375
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
[ruff
] Add analyze.string-imports-min-dots
to settings
#20375
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good but I think we should rename the option to match the CLI
root.child("ruff.toml").write_str(indoc::indoc! {r#" | ||
[analyze] | ||
detect-string-imports = true | ||
string-imports-min-dots = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be min-dots
for consistency with the CLI
Options:
--direction <DIRECTION> The direction of the import map. By default, generates a dependency map, i.e., a map from file to files that it depends on. Use `--direction
dependents` to generate a map from file to files that depend on it [default: dependencies] [possible values: dependencies, dependents]
--detect-string-imports Attempt to detect imports from string literals
--min-dots <MIN_DOTS> The minimum number of dots in a string import to consider it a valid import
--preview Enable preview mode. Use `--no-preview` to disable
--target-version <TARGET_VERSION> The minimum Python version that should be supported [possible values: py37, py38, py39, py310, py311, py312, py313, py314]
--python <PYTHON> Path to a virtual environment to use for resolving additional dependencies
-h, --help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I have renamed it to min-dots
!
ruff
] Add analyze.string-imports-min-dots
to settings docsruff
] Add analyze.min-dots
to settings docs
ruff
] Add analyze.min-dots
to settings docsruff
] Add analyze.min-dots
to settings
Oh wait, I made a mistake. I assumed you added the new option but that isn't the case. This is a pre-existing option (if I understand your summary correctly) and this PR only adds documentation. If that's the case, then we shouldn't rename the option because that would break existing users |
ruff
] Add analyze.min-dots
to settingsruff
] Add analyze.string-imports-min-dots
to settings
18e61e6
to
9e0ad2d
Compare
@MichaReiser Thanks for the clarification. I had the same concern, so I didn’t rename the option; I’ll keep the existing name as is. |
Summary
Fixes #20110
string_imports_min_dots
was introduced in #19538, so the option has been available since then; this PR adds documentation and a config test.Test Plan
Add integration test
string_detection_from_config
to verify the config setting works.