From 9ca1f0003fa33e181945d29d8c84f73abd50fc83 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 23 Oct 2024 12:14:58 -0400 Subject: [PATCH] Fix `uv add` comment handling for empty arrays (#8504) ## Summary Closes https://github.com/astral-sh/uv/issues/8496. --- crates/uv-workspace/src/pyproject_mut.rs | 27 ++- crates/uv/tests/it/edit.rs | 237 +++++++++++++++++++++++ 2 files changed, 259 insertions(+), 5 deletions(-) diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index 7f5b3e9cf..76ff3da25 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -366,7 +366,6 @@ impl PyProjectTomlMut { // Set the position to the minimum, if it's not already the first element. if let Some(min) = existing.iter().filter_map(toml_edit::Table::position).min() { - // if !table.position().is_some_and(|position| position < min) { table.set_position(min); // Increment the position of all existing elements. @@ -375,7 +374,6 @@ impl PyProjectTomlMut { table.set_position(position + 1); } } - // } } // Push the item to the table. @@ -805,11 +803,16 @@ pub fn add_dependency( let index = index.unwrap_or(deps.len()); let mut value = Value::from(req_string.as_str()); + let decor = value.decor_mut(); - decor.set_prefix(deps.trailing().clone()); - deps.set_trailing(""); + + if index == deps.len() { + decor.set_prefix(deps.trailing().clone()); + deps.set_trailing(""); + } deps.insert_formatted(index, value); + // `reformat_array_multiline` uses the indentation of the first dependency entry. // Therefore, we retrieve the indentation of the first dependency entry and apply it to // the new entry. Note that it is only necessary if the newly added dependency is going @@ -992,6 +995,11 @@ fn reformat_array_multiline(deps: &mut Array) { .and_then(|s| s.lines().last()) .unwrap_or_default(); + let decor_prefix = decor_prefix + .split_once('#') + .map(|(s, _)| s) + .unwrap_or(decor_prefix); + // If there is no indentation, use four-space. indentation_prefix = Some(if decor_prefix.is_empty() { " ".to_string() @@ -1023,7 +1031,16 @@ fn reformat_array_multiline(deps: &mut Array) { let mut rv = String::new(); if comments.peek().is_some() { for comment in comments { - rv.push_str("\n "); + match comment.comment_type { + CommentType::OwnLine => { + let indentation_prefix_str = + format!("\n{}", indentation_prefix.as_ref().unwrap()); + rv.push_str(&indentation_prefix_str); + } + CommentType::EndOfLine => { + rv.push(' '); + } + } rv.push_str(&comment.text); } } diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index 01d9a1059..0867b96e7 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -6591,3 +6591,240 @@ fn add_preserves_open_bracket_comment() -> Result<()> { }); Ok(()) } + +#[test] +fn add_preserves_empty_comment() -> 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" + requires-python = ">=3.12" + dependencies = [ + # First line. + # Second line. + ] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "#})?; + + uv_snapshot!(context.filters(), context.add().arg("anyio==3.7.0"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + Prepared 4 packages in [TIME] + Installed 4 packages in [TIME] + + anyio==3.7.0 + + idna==3.6 + + project==0.1.0 (from file://[TEMP_DIR]/) + + sniffio==1.3.1 + "###); + + 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" + requires-python = ">=3.12" + dependencies = [ + # First line. + # Second line. + "anyio==3.7.0", + ] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "### + ); + }); + + Ok(()) +} + +#[test] +fn add_preserves_trailing_comment() -> 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" + requires-python = ">=3.12" + dependencies = [ + "idna", + "iniconfig", # Use iniconfig. + # First line. + # Second line. + ] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "#})?; + + uv_snapshot!(context.filters(), context.add().arg("anyio==3.7.0"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 5 packages in [TIME] + Prepared 5 packages in [TIME] + Installed 5 packages in [TIME] + + anyio==3.7.0 + + idna==3.6 + + iniconfig==2.0.0 + + project==0.1.0 (from file://[TEMP_DIR]/) + + sniffio==1.3.1 + "###); + + 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" + requires-python = ">=3.12" + dependencies = [ + "anyio==3.7.0", + "idna", + "iniconfig", # Use iniconfig. + # First line. + # Second line. + ] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "### + ); + }); + + uv_snapshot!(context.filters(), context.add().arg("typing-extensions"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 6 packages in [TIME] + Prepared 2 packages in [TIME] + Uninstalled 1 package in [TIME] + Installed 2 packages in [TIME] + ~ project==0.1.0 (from file://[TEMP_DIR]/) + + typing-extensions==4.10.0 + "###); + + 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" + requires-python = ">=3.12" + dependencies = [ + "anyio==3.7.0", + "idna", + "iniconfig", # Use iniconfig. + # First line. + # Second line. + "typing-extensions>=4.10.0", + ] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "### + ); + }); + + Ok(()) +} + +#[test] +fn add_preserves_trailing_depth() -> 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" + requires-python = ">=3.12" + dependencies = [ + "idna", + "iniconfig",# Use iniconfig. + # First line. + # Second line. + ] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "#})?; + + uv_snapshot!(context.filters(), context.add().arg("anyio==3.7.0"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 5 packages in [TIME] + Prepared 5 packages in [TIME] + Installed 5 packages in [TIME] + + anyio==3.7.0 + + idna==3.6 + + iniconfig==2.0.0 + + project==0.1.0 (from file://[TEMP_DIR]/) + + sniffio==1.3.1 + "###); + + 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" + requires-python = ">=3.12" + dependencies = [ + "anyio==3.7.0", + "idna", + "iniconfig", # Use iniconfig. + # First line. + # Second line. + ] + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "### + ); + }); + + Ok(()) +}