Sort dependency group keys when adding new group #11591
Conversation
However, if earlier keys were not sorted, we just append to avoid churn.
jtfmumm
force-pushed
the
jtfm/sorted-entries
branch
from
2ee5ea8 to
58fa23f
Compare
crates/uv/tests/it/edit.rs
Outdated
| @@ -4826,6 +4826,112 @@ fn add_group() -> Result<()> { | |||
|
|
|||
| let pyproject_toml = context.read("pyproject.toml"); | |||
|
|
|||
| insta::with_settings!({ | |||
There was a problem hiding this comment.
Can we add a few test cases where there are comments between the groups, to understand how they move around?
There was a problem hiding this comment.
👍 this is an unfortunately frequent cause of bug reports
There was a problem hiding this comment.
Added two new tests
crates/uv/tests/it/edit.rs
Outdated
| @@ -4826,6 +4826,112 @@ fn add_group() -> Result<()> { | |||
|
|
|||
| let pyproject_toml = context.read("pyproject.toml"); | |||
|
|
|||
| insta::with_settings!({ | |||
| filters => context.filters(), | |||
There was a problem hiding this comment.
Random, but it doesn't actually look like you need the filters for this snapshot. You could reduce nesting by omitting them — but I don't mind either way.
There was a problem hiding this comment.
Removed filters on these tests
| [dependency-groups] | ||
| alpha = [ | ||
| "anyio==3.7.0", | ||
| ] | ||
| # This is our dev group | ||
| dev = [ | ||
| "anyio==3.7.0", | ||
| ] |
There was a problem hiding this comment.
Could sort default-groups separately and put them in top-most priority?
(it is implicitly set to tool.uv.default-groups = ["dev"])
There was a problem hiding this comment.
I think I'd prefer not to do that, otherwise when you change your default groups the ordering is unsorted and we won't respect it anymore. Though it is interesting.
There was a problem hiding this comment.
Correct, I would like to see it as a lint rule in future.
| .iter() | ||
| .filter_map(|(dotted_ks, _)| dotted_ks.first()) | ||
| .map(|k| k.get()) | ||
| .is_sorted(); |
There was a problem hiding this comment.
The logic we use for detecting sorted dependencies is a bit more complex. Specifically, we check whether the values are sorted case-insensitively or case-sensitively, and then respect that sort if so. We could do the same here, but I don't consider it blocking (it seems pretty rare for users to use capitalization in these identifiers, unlike in package names).
This change keeps dependency group keys sorted when adding new ones.
If earlier dependency group keys were not sorted, we just append the new group key to avoid churn in
pyproject.toml. See discussion on #11447. I've added a new snapshot test to capture this case.Closes #11447.