Skip to content

Added config file settings validation - #1

Closed
j-helland wants to merge 2 commits into
jwh/toml-config-preview-featuresfrom
jwh/config-file-validation
Closed

Added config file settings validation#1
j-helland wants to merge 2 commits into
jwh/toml-config-preview-featuresfrom
jwh/config-file-validation

Conversation

@j-helland

Copy link
Copy Markdown
Owner

Relates to astral-sh#16452

Summary

Per discussion astral-sh#16452 (comment), it was determined that we need to have additional config validation for the preview and preview-features fields.

This is a sketch of how we might approach that via a Validator trait implemented for Options and GlobalOptions.

Test Plan

New and existing automated tests.

…e fields

* Instead of merging commandline `--preview-features` with config file `preview-features`, now the commandline arguments take precedence. This matches the semantics of how uv handles other commandline arguments.
* Removed unnecessary `PreviewFeaturesMode` type.
* Fixed bug with `PreviewFeatures` serde serializer that produced a list of lists instead of a flat list as expected.
@j-helland
j-helland force-pushed the jwh/config-file-validation branch from 3d6b8cf to 59fd4f9 Compare December 21, 2025 01:41
Specifically, disallow specifying both `preview` and `preview-features` in the config files. The implementation is intended to be extensible, should more validation rules be needed in the future.
@j-helland
j-helland force-pushed the jwh/config-file-validation branch from 59fd4f9 to ab5f1cd Compare December 21, 2025 01:49
Comment on lines +7 to +9
pub(crate) trait Validator<Context = (), Err = ValidationError> {
fn validate(&self, ctx: &Context) -> Result<(), Err>;
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This trait might be a bit over-engineered. If we think that there will be lots of additional validation rules in the future, something like this makes sense. If not, simplifying to something like validate_uv_toml would be better.

YAGNI?

Comment on lines +53 to 54
options.validate(&Context { path: &file })?;
validate_uv_toml(&file, &options)?;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I didn't consolidate validate_uv_toml into the validator trait impl because it actually doesn't get called in the pyproject.toml branch.

@j-helland
j-helland marked this pull request as ready for review December 21, 2025 01:50
@j-helland j-helland closed this Dec 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant