Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 42 additions & 11 deletions crates/uv-workspace/src/pyproject_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,26 +533,57 @@ pub fn add_dependency(

match to_replace.as_slice() {
[] => {
#[derive(Debug, Copy, Clone)]
enum Sort {
/// The list is sorted in a case-sensitive manner.
CaseSensitive,
/// The list is sorted in a case-insensitive manner.
CaseInsensitive,
/// The list is unsorted.
Unsorted,
}

// Determine if the dependency list is sorted prior to
// adding the new dependency; the new dependency list
// will be sorted only when the original list is sorted
// so that user's custom dependency ordering is preserved.
//
// Additionally, if the table is invalid (i.e. contains non-string values)
// we still treat it as unsorted for the sake of simplicity.
let sorted = deps.iter().all(toml_edit::Value::is_str)
&& deps
.iter()
.tuple_windows()
.all(|(a, b)| a.as_str() <= b.as_str());
//
// We account for both case-sensitive and case-insensitive sorting.
let sort = deps
.iter()
.all(Value::is_str)
.then(|| {
if deps.iter().tuple_windows().all(|(a, b)| {
a.as_str().map(str::to_lowercase) <= b.as_str().map(str::to_lowercase)
}) {
Some(Sort::CaseInsensitive)
} else if deps
.iter()
.tuple_windows()
.all(|(a, b)| a.as_str() <= b.as_str())
{
Some(Sort::CaseSensitive)
} else {
None
}
})
.flatten()
.unwrap_or(Sort::Unsorted);

let req_string = req.to_string();
let index = if sorted {
deps.iter()
.position(|d: &Value| d.as_str() > Some(req_string.as_str()))
.unwrap_or(deps.len())
} else {
deps.len()
let index = match sort {
Sort::CaseSensitive => deps
.iter()
.position(|d| d.as_str() > Some(req_string.as_str())),
Sort::CaseInsensitive => deps.iter().position(|d| {
d.as_str().map(str::to_lowercase) > Some(req_string.as_str().to_lowercase())
}),
Sort::Unsorted => None,
};
let index = index.unwrap_or(deps.len());

deps.insert(index, req_string);
// `reformat_array_multiline` uses the indentation of the first dependency entry.
Expand Down
69 changes: 69 additions & 0 deletions crates/uv/tests/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4680,6 +4680,74 @@ fn sorted_dependencies() -> Result<()> {

let pyproject_toml = context.read("pyproject.toml");

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
pyproject_toml, @r###"
[project]
name = "project"
version = "0.1.0"
description = "Add your description here"
requires-python = ">=3.12"
dependencies = [
"anyio>=4.3.0",
"CacheControl[filecache]>=0.14,<0.15",
Copy link
Member

Choose a reason for hiding this comment

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

I think we might want to preserve either case-sensitive or case-insensitive sorts, rather than always using a case-insensitive sort. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean, I'd probably prefer case insensitive sort as that's probably the most common case. In this example, I'd expect anyio to come before CacheControl. But happy to do it either way.

Copy link
Member

Choose a reason for hiding this comment

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

I'm happy to default to case insensitive, but if the user has intentionally sorted as case sensitive, I think it makes sense to preserve it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I've also added a test for the case-sensitive case.

"iniconfig",
"typing-extensions>=4.10.0",
]
"###
);
});
Ok(())
}

/// Ensure that the added dependencies are case sensitive sorted if the dependency list was already
/// case sensitive sorted prior to the operation.
#[test]
fn case_sensitive_sorted_dependencies() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(indoc! {r#"
[project]
name = "project"
version = "0.1.0"
description = "Add your description here"
requires-python = ">=3.12"
dependencies = [
"CacheControl[filecache]>=0.14,<0.15",
"PyYAML",
"iniconfig",
]
"#})?;

uv_snapshot!(context.filters(), context.add().args(["typing-extensions", "anyio"]), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
Resolved 14 packages in [TIME]
Prepared 13 packages in [TIME]
Installed 13 packages in [TIME]
+ anyio==4.3.0
+ cachecontrol==0.14.0
+ certifi==2024.2.2
+ charset-normalizer==3.3.2
+ filelock==3.13.1
+ idna==3.6
+ iniconfig==2.0.0
+ msgpack==1.0.8
+ pyyaml==6.0.1
+ requests==2.31.0
+ sniffio==1.3.1
+ typing-extensions==4.10.0
+ urllib3==2.2.1
"###);

let pyproject_toml = context.read("pyproject.toml");

insta::with_settings!({
filters => context.filters(),
}, {
Expand All @@ -4692,6 +4760,7 @@ fn sorted_dependencies() -> Result<()> {
requires-python = ">=3.12"
dependencies = [
"CacheControl[filecache]>=0.14,<0.15",
"PyYAML",
"anyio>=4.3.0",
"iniconfig",
"typing-extensions>=4.10.0",
Expand Down