From f9c800be9ae9d7143cc090d4e6fd55e0ae281cbd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 13 May 2026 14:58:25 +0000 Subject: [PATCH 1/2] feat: support TOML 1.1 multiline inline tables Bump toml_edit to 0.25 (toml and toml-span already support TOML 1.1 on main). Add insert_into_inline_table_with_format to preserve multiline formatting, indentation style, and trailing comma conventions when inserting new entries. Includes comprehensive tests: direct unit tests for the formatting function (trailing commas, sequential insertions, empty tables, tab indentation, single-line fallback, existing key updates, non-dependency tables), integration tests through TomlDocument, and consolidated parse-only tests for upstream TOML 1.1 support. All tests use inline insta snapshots. https://claude.ai/code/session_01FT59TTz4yRy3DPwbREE82u --- Cargo.lock | 34 +-- Cargo.toml | 2 +- crates/pixi_manifest/src/toml/document.rs | 317 +++++++++++++++++++++- crates/pixi_manifest/src/toml/manifest.rs | 121 +++++++++ docs/reference/pixi_manifest.md | 19 ++ 5 files changed, 463 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7cb946b793..11d7c04c6d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5848,7 +5848,7 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "tokio", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "tracing-subscriber", "typed-path", @@ -6309,7 +6309,7 @@ dependencies = [ "thiserror 2.0.18", "tokio", "tokio-util 0.7.18", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "tracing-subscriber", "typed-path", @@ -6498,7 +6498,7 @@ dependencies = [ "serde_ignored", "serde_json", "thiserror 2.0.18", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "url", ] @@ -6587,7 +6587,7 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "tokio", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "tracing-test", "typed-path", @@ -6742,7 +6742,7 @@ dependencies = [ "thiserror 2.0.18", "tokio", "toml-span", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "url", "zstd", @@ -6849,7 +6849,7 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "toml-span", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "url", ] @@ -6899,7 +6899,7 @@ dependencies = [ "serde_json", "thiserror 2.0.18", "toml-span", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "url", ] @@ -6991,7 +6991,7 @@ dependencies = [ "serde_with", "thiserror 2.0.18", "toml-span", - "toml_edit 0.23.10+spec-1.0.0", + "toml_edit 0.25.11+spec-1.1.0", "tracing", "typed-path", "url", @@ -11041,21 +11041,6 @@ dependencies = [ "serde_core", ] -[[package]] -name = "toml_edit" -version = "0.23.10+spec-1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "84c8b9f757e028cee9fa244aea147aab2a9ec09d5325a9b01e0a49730c2b5269" -dependencies = [ - "indexmap 2.14.0", - "serde_core", - "serde_spanned", - "toml_datetime 0.7.5+spec-1.1.0", - "toml_parser", - "toml_writer", - "winnow 0.7.15", -] - [[package]] name = "toml_edit" version = "0.24.1+spec-1.1.0" @@ -11078,8 +11063,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b59c4d22ed448339746c59b905d24568fcbb3ab65a500494f7b8c3e97739f2b" dependencies = [ "indexmap 2.14.0", + "serde_core", + "serde_spanned", "toml_datetime 1.1.1+spec-1.1.0", "toml_parser", + "toml_writer", "winnow 1.0.2", ] diff --git a/Cargo.toml b/Cargo.toml index 09f52dbb87..9ab6baf23b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -175,7 +175,7 @@ tokio-test = "0.4.5" tokio-util = "0.7.13" toml = "1.0.0" toml-span = "0.7.0" -toml_edit = "0.23.0" +toml_edit = "0.25.0" tracing = "0.1.41" xz2 = "0.1.7" # Forcing the version due to this PR https://github.com/tokio-rs/tracing/pull/3368 diff --git a/crates/pixi_manifest/src/toml/document.rs b/crates/pixi_manifest/src/toml/document.rs index f98f1b7d36..baca29696f 100644 --- a/crates/pixi_manifest/src/toml/document.rs +++ b/crates/pixi_manifest/src/toml/document.rs @@ -3,7 +3,7 @@ use std::{ fmt::{Display, Formatter}, }; -use toml_edit::{Array, Item, Table, TableLike, Value}; +use toml_edit::{Array, Decor, InlineTable, Item, Key, Table, TableLike, Value}; use crate::TomlError; @@ -121,11 +121,13 @@ impl TomlDocument { // Add dependency as inline table if it doesn't exist if let Some(dependency) = current_table.get_mut(last_key) { - dependency - .as_table_like_mut() - .map(|table| table.insert(key, Item::Value(value))); + if let Some(inline_table) = dependency.as_inline_table_mut() { + insert_into_inline_table_with_format(inline_table, key, value); + } else if let Some(table) = dependency.as_table_like_mut() { + table.insert(key, Item::Value(value)); + } } else { - let mut dependency = toml_edit::InlineTable::new(); + let mut dependency = InlineTable::new(); dependency.insert(key, value); current_table.insert(last_key, toml_edit::value(dependency)); } @@ -161,14 +163,80 @@ impl TomlDocument { } } +/// Inserts a key-value pair into an inline table, respecting multiline +/// formatting. If the existing entries use newlines (i.e. the table is a TOML +/// 1.1 multiline inline table), the new entry is placed on its own line with +/// matching indentation. The trailing comma style of the table is preserved. +fn insert_into_inline_table_with_format(table: &mut InlineTable, key: &str, value: Value) { + // Detect whether the table is multiline by checking if any existing key + // has a newline in its prefix decoration. + let indentation = table + .iter() + .filter_map(|(k, _)| { + let prefix = table.key(k)?.leaf_decor().prefix()?.as_str()?; + if prefix.contains('\n') { + // Extract the indentation after the last newline + let indent = prefix.rsplit_once('\n').map_or(prefix, |(_, after)| after); + Some(indent.to_owned()) + } else { + None + } + }) + .next(); + + if let Some(indent) = indentation { + let had_trailing_comma = table.trailing_comma(); + + // Ensure the last existing value has a clean suffix so the comma + // sits right after the value (no ` ,` artifacts). + if let Some((last_key, _)) = table.iter().last() { + let last_key = last_key.to_owned(); + if let Some(val) = table.get_mut(&last_key) { + let suffix = val.decor().suffix().and_then(|s| s.as_str()).unwrap_or(""); + if suffix.trim().is_empty() { + val.decor_mut().set_suffix(""); + } + } + } + + let formatted_key = Key::new(key).with_leaf_decor(Decor::new(format!("\n{indent}"), " ")); + let mut formatted_value = value; + // Clear any default suffix on the value so there is no space before + // a potential trailing comma. + formatted_value.decor_mut().set_suffix(""); + table.insert_formatted(&formatted_key, formatted_value); + + table.set_trailing_comma(had_trailing_comma); + + // Ensure the closing `}` stays on its own line by preserving the + // trailing whitespace that precedes it. + let trailing = table.trailing().as_str().unwrap_or(""); + if !trailing.contains('\n') { + table.set_trailing("\n".to_string()); + } + } else { + table.insert(key, value); + } +} + #[cfg(test)] mod tests { use std::str::FromStr; - use toml_edit::{DocumentMut, Item}; + use toml_edit::{DocumentMut, InlineTable, Item}; + use super::insert_into_inline_table_with_format; use crate::toml::document::TomlDocument; + /// Helper to parse a bare inline table string like `{ foo = "bar" }` into + /// an [`InlineTable`], so we can test `insert_into_inline_table_with_format` + /// directly without needing a full `[section]` header. + fn parse_inline_table(s: &str) -> InlineTable { + let doc = format!("t = {s}"); + let doc = DocumentMut::from_str(&doc).unwrap(); + doc.as_table()["t"].as_inline_table().unwrap().clone() + } + #[test] fn test_get_or_insert_nested_table() { let toml = r#" @@ -349,4 +417,241 @@ dependencies = { dummy = "3.11.*" } // Verify the dependency was added as inline table assert!(result.contains("dependencies = { rich = \"*\" }")); } + + /// Tests that multiline inline tables (TOML 1.1) can be parsed by + /// toml_edit and round-trip correctly. + #[test] + fn test_multiline_inline_table_parsing() { + let toml = r#" +[envs.python] +channels = ["dummy-channel"] +dependencies = { + dummy = "3.11.*", + other = "1.0", +} +"#; + let manifest = TomlDocument::new(DocumentMut::from_str(toml).unwrap()); + insta::assert_snapshot!(manifest.0.to_string(), @r#" + + [envs.python] + channels = ["dummy-channel"] + dependencies = { + dummy = "3.11.*", + other = "1.0", + } + "#); + } + + // ---- Direct tests for insert_into_inline_table_with_format ---- + + /// Single-entry multiline WITH trailing comma (inverse of the Hofer-Julian + /// bug case). + #[test] + fn test_format_single_entry_with_trailing_comma() { + let mut table = parse_inline_table("{\n numpy = \"*\",\n}"); + insert_into_inline_table_with_format(&mut table, "scipy", toml_edit::Value::from(">=1.0")); + insta::assert_snapshot!(table.to_string(), @r#" + { + numpy = "*", + scipy = ">=1.0", + } + "#); + } + + /// Sequential insertions — insert two entries one after another into the + /// same table (the real-world `pixi add foo && pixi add bar` scenario). + #[test] + fn test_format_sequential_insertions() { + let mut table = parse_inline_table("{\n numpy = \"*\",\n}"); + insert_into_inline_table_with_format(&mut table, "scipy", toml_edit::Value::from(">=1.0")); + insert_into_inline_table_with_format(&mut table, "pandas", toml_edit::Value::from(">=2.0")); + insta::assert_snapshot!(table.to_string(), @r#" + { + numpy = "*", + scipy = ">=1.0", + pandas = ">=2.0", + } + "#); + } + + /// Empty multiline table — verifies fallback to plain insert. + #[test] + fn test_format_empty_table() { + let mut table = parse_inline_table("{}"); + insert_into_inline_table_with_format(&mut table, "foo", toml_edit::Value::from("1.0")); + insta::assert_snapshot!(table.to_string(), @r#" { foo = "1.0" }"#); + } + + /// Tab indentation — verifies tabs are preserved, not converted to spaces. + #[test] + fn test_format_tab_indentation() { + let mut table = parse_inline_table("{\n\tnumpy = \"*\",\n}"); + insert_into_inline_table_with_format(&mut table, "scipy", toml_edit::Value::from(">=1.0")); + insta::assert_snapshot!(table.to_string(), @r#" + { + numpy = "*", + scipy = ">=1.0", + } + "#); + } + + /// Single-line table through the format function — verifies the else branch + /// (non-multiline) preserves single-line formatting. + #[test] + fn test_format_single_line_table() { + let mut table = parse_inline_table("{ numpy = \"*\" }"); + insert_into_inline_table_with_format(&mut table, "scipy", toml_edit::Value::from(">=1.0")); + insta::assert_snapshot!(table.to_string(), @r#" { numpy = "*" , scipy = ">=1.0" }"#); + } + + /// Empty multiline table with newline (`{\n}`) — a valid TOML 1.1 construct + /// with no entries but containing whitespace. No key has a newline prefix, + /// so it should fall back to plain insert. + #[test] + fn test_format_empty_multiline_table() { + let mut table = parse_inline_table("{\n}"); + insert_into_inline_table_with_format(&mut table, "foo", toml_edit::Value::from("1.0")); + // Normalize trailing whitespace (toml_edit Display artifact) for snapshot + let result = table + .to_string() + .lines() + .map(|l| l.trim_end()) + .collect::>() + .join("\n"); + insta::assert_snapshot!(result, @r#" + { foo = "1.0" + } + "#); + } + + /// Updating an existing key in a multiline inline table — verifies that + /// re-inserting a key that already exists replaces the value without + /// corrupting formatting. + #[test] + fn test_format_update_existing_key() { + let mut table = parse_inline_table("{\n numpy = \"1.0\",\n scipy = \"2.0\",\n}"); + insert_into_inline_table_with_format(&mut table, "numpy", toml_edit::Value::from(">=2.0")); + insta::assert_snapshot!(table.to_string(), @r#" + { + numpy = ">=2.0", + scipy = "2.0", + } + "#); + } + + /// Non-dependency inline table — verifies the function works on any inline + /// table, not just dependencies (e.g. a build backend spec). + #[test] + fn test_format_non_dependency_inline_table() { + let mut table = + parse_inline_table("{\n name = \"pixi-build-python\",\n version = \"*\",\n}"); + insert_into_inline_table_with_format( + &mut table, + "channel", + toml_edit::Value::from("conda-forge"), + ); + insta::assert_snapshot!(table.to_string(), @r#" + { + name = "pixi-build-python", + version = "*", + channel = "conda-forge", + } + "#); + } + + /// Tests that inserting into a multiline inline table preserves the + /// multiline format. + #[test] + fn test_insert_into_multiline_inline_table() { + let toml = r#" +[envs.python] +channels = ["dummy-channel"] +dependencies = { + dummy = "3.11.*", + other = "1.0", +} +"#; + let mut manifest = TomlDocument::new(DocumentMut::from_str(toml).unwrap()); + manifest + .insert_into_inline_table( + &["envs", "python", "dependencies"], + "new-pkg", + toml_edit::Value::from("2.0"), + ) + .unwrap(); + + insta::assert_snapshot!(manifest.0.to_string(), @r#" + + [envs.python] + channels = ["dummy-channel"] + dependencies = { + dummy = "3.11.*", + other = "1.0", + new-pkg = "2.0", + } + "#); + } + + /// Tests inserting into a multiline inline table where the last element + /// does NOT have a trailing comma. The new entry should not add one either. + #[test] + fn test_insert_into_multiline_inline_table_no_trailing_comma() { + let toml = r#" +[envs.python] +channels = ["dummy-channel"] +dependencies = { + dummy = "3.11.*", + other = "1.0" +} +"#; + let mut manifest = TomlDocument::new(DocumentMut::from_str(toml).unwrap()); + manifest + .insert_into_inline_table( + &["envs", "python", "dependencies"], + "new-pkg", + toml_edit::Value::from("2.0"), + ) + .unwrap(); + + insta::assert_snapshot!(manifest.0.to_string(), @r#" + + [envs.python] + channels = ["dummy-channel"] + dependencies = { + dummy = "3.11.*", + other = "1.0", + new-pkg = "2.0" + } + "#); + } + + /// Reproduces the bug from https://github.com/prefix-dev/pixi/pull/5655#issuecomment-4040362659 + /// where inserting into a single-entry multiline inline table without a + /// trailing comma produces `{ numpy = "*"\n, pydantic = ... }`. + #[test] + fn test_insert_into_multiline_inline_table_single_entry() { + let toml = r#" +[feature.test] +dependencies = { + numpy = "*" +} +"#; + let mut manifest = TomlDocument::new(DocumentMut::from_str(toml).unwrap()); + manifest + .insert_into_inline_table( + &["feature", "test", "dependencies"], + "pydantic", + toml_edit::Value::from(">=2.12.5,<3"), + ) + .unwrap(); + + insta::assert_snapshot!(manifest.0.to_string(), @r#" + + [feature.test] + dependencies = { + numpy = "*", + pydantic = ">=2.12.5,<3" + } + "#); + } } diff --git a/crates/pixi_manifest/src/toml/manifest.rs b/crates/pixi_manifest/src/toml/manifest.rs index 96afc22940..2fe681ba22 100644 --- a/crates/pixi_manifest/src/toml/manifest.rs +++ b/crates/pixi_manifest/src/toml/manifest.rs @@ -1258,6 +1258,127 @@ mod test { )); } + /// Tests that TOML 1.1 multiline inline tables are parsed correctly. + /// TOML 1.1 allows inline tables to span multiple lines with trailing + /// commas. Verify that various forms parse successfully (the upstream + /// `toml_edit` crate handles this). Each sub-case triggers `unwrap()`, + /// which will panic if parsing fails. + #[test] + fn test_multiline_inline_tables_parsing() { + // Basic multiline inline table with trailing comma in dependencies + WorkspaceManifest::from_toml_str_with_base_dir( + r#" + [workspace] + name = "test-multiline-inline" + channels = ["conda-forge"] + platforms = ["linux-64"] + + [dependencies] + python = { + version = ">=3.12", + channel = "conda-forge", + } + + [feature.test.dependencies] + pytest = { + version = ">=7.0", + channel = "conda-forge", + } + + [environments] + test = ["test"] + "#, + Path::new(""), + ) + .unwrap(); + + // Deeply nested multiline inline table (e.g. package.build.backend) + WorkspaceManifest::from_toml_str_with_base_dir( + r#" + [workspace] + name = "test-nested-multiline" + channels = ["conda-forge"] + platforms = ["linux-64"] + preview = ["pixi-build"] + + [package] + name = "test-nested-multiline" + version = "0.1.0" + + [package.build] + backend = { + name = "pixi-build-python", + version = "*", + } + "#, + Path::new(""), + ) + .unwrap(); + + // Multiline arrays combined with multiline inline table + WorkspaceManifest::from_toml_str_with_base_dir( + r#" + [workspace] + name = "test-trailing-comma" + channels = [ + "conda-forge", + ] + platforms = [ + "linux-64", + "osx-arm64", + ] + + [dependencies] + numpy = { + version = ">=1.24", + } + "#, + Path::new(""), + ) + .unwrap(); + } + + /// Tests that error spans are correctly reported inside multiline inline + /// tables (TOML 1.1). The span should point to the invalid value itself, + /// not just the surrounding table. + #[test] + fn test_multiline_inline_table_error_span() { + assert_snapshot!(expect_parse_failure( + r#" + [workspace] + name = "test-error-span" + channels = [] + platforms = ["linux-64"] + + [dependencies] + bad-pkg = { + version = "!invalid!", + channel = "conda-forge", + } + "#, + ), @r#" + × 0: at line 1, in Tag: + │ !invalid! + │ ^ + + │ 1: at line 1, in Alt: + │ !invalid! + │ ^ + + │ 2: at line 1, in version: + │ !invalid! + │ ^ + + + ╭─[pixi.toml:9:24] + 8 │ bad-pkg = { + 9 │ version = "!invalid!", + · ───────── + 10 │ channel = "conda-forge", + ╰──── + "#); + } + #[test] fn test_project_deprecation_warning() { assert_snapshot!( diff --git a/docs/reference/pixi_manifest.md b/docs/reference/pixi_manifest.md index 364f0821ca..460847602d 100644 --- a/docs/reference/pixi_manifest.md +++ b/docs/reference/pixi_manifest.md @@ -6,6 +6,25 @@ those environments. It can also specify tasks which can run in those environment A `toml` file is structured in different tables. This document will explain the usage of the different tables. +!!! note "TOML 1.1 support" + Pixi supports the [TOML 1.1 specification](https://toml.io/en/v1.1.0), which introduces multiline inline tables and trailing commas. + This means you can write inline tables across multiple lines for better readability: + + ```toml + [dependencies] + python = { + version = ">=3.12", + channel = "conda-forge", + } + ``` + + This is equivalent to the single-line form: + + ```toml + [dependencies] + python = { version = ">=3.12", channel = "conda-forge" } + ``` + !!! tip We also support the `pyproject.toml` file. It has the same structure as the `pixi.toml` file. except that you need to prepend the tables with `tool.pixi` instead of just the table name. For example, the `[workspace]` table becomes `[tool.pixi.workspace]`. From 07aaa9c38243c1901f5c324f96cc6ab31e8ae52e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 13 May 2026 15:39:45 +0000 Subject: [PATCH 2/2] feat: format-preserving add_dependency for multiline inline tables Route new dependency insertions through insert_into_inline_table so that multiline inline tables (TOML 1.1) keep their layout, indentation, and trailing-comma style when running `pixi add`. The same fix is applied to add_pypi_dependency in both the pixi-native and reuse-existing-feature-table code paths. https://claude.ai/code/session_01FT59TTz4yRy3DPwbREE82u --- .../pixi_manifest/src/manifests/document.rs | 306 +++++++++++++++--- ...ency_preserves_multiline_inline_table.snap | 15 + ...tiline_inline_table_no_trailing_comma.snap | 15 + ...ency_update_in_multiline_inline_table.snap | 14 + ...ency_preserves_multiline_inline_table.snap | 15 + ...ncy_reuse_feature_preserves_multiline.snap | 17 + 6 files changed, 329 insertions(+), 53 deletions(-) create mode 100644 crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table.snap create mode 100644 crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table_no_trailing_comma.snap create mode 100644 crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_update_in_multiline_inline_table.snap create mode 100644 crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_preserves_multiline_inline_table.snap create mode 100644 crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_reuse_feature_preserves_multiline.snap diff --git a/crates/pixi_manifest/src/manifests/document.rs b/crates/pixi_manifest/src/manifests/document.rs index 66e7022cdc..53424b11da 100644 --- a/crates/pixi_manifest/src/manifests/document.rs +++ b/crates/pixi_manifest/src/manifests/document.rs @@ -342,12 +342,14 @@ impl ManifestDocument { .with_feature_name(Some(feature_name)) .with_table(Some(spec_type.name())); - self.manifest_mut() - .get_or_insert_nested_table(&dependency_table.as_keys()) - .map(|t| { - let mut new_value = spec.to_toml_value(); + let keys = dependency_table.as_keys(); + let new_value = spec.to_toml_value(); - // Check if there is an existing entry that is represented by an inline value. + // Try to find and update an existing entry, preserving its decoration. + let updated = self + .manifest_mut() + .get_or_insert_nested_table(&keys) + .map(|t| { let existing_value = t.iter_mut().find_map(|(key, value)| { let package_key_name = PackageName::from_str(key.get()).ok()?; if package_key_name == *name { @@ -357,18 +359,23 @@ impl ManifestDocument { } }); - // If there exists an existing value, we update it with the new value, but we - // keep the decoration. if let Some(existing_value) = existing_value { - *new_value.decor_mut() = existing_value.decor().clone(); - *existing_value = new_value; + let mut cloned = new_value.clone(); + *cloned.decor_mut() = existing_value.decor().clone(); + *existing_value = cloned; + true } else { - // Otherwise, just reinsert the value. This might overwrite an existing - // decorations. - t.insert(name.as_normalized(), Item::Value(new_value)); + false } })?; + if !updated { + // Insert via the format-preserving path so multiline inline tables + // keep their layout. + self.manifest_mut() + .insert_into_inline_table(&keys, name.as_normalized(), new_value)?; + } + Ok(()) } @@ -410,29 +417,39 @@ impl ManifestDocument { .with_feature_name(Some(feature_name)) .with_table(Some(consts::PYPI_DEPENDENCIES)); - let table = self - .manifest_mut() - .get_or_insert_nested_table(&dependency_table_name.as_keys())?; - - let mut new_value = Value::from(pypi_requirement); + let keys = dependency_table_name.as_keys(); + let new_value = Value::from(pypi_requirement); - // Check if there exists an existing entry in the table that we should overwrite - // instead. - let existing_value = table.iter_mut().find_map(|(key, value)| { - let existing_name = pep508_rs::PackageName::from_str(key.get()).ok()?; - if existing_name == requirement.name { - value.as_value_mut() - } else { - None - } - }); - - // If there exists an existing entry, we overwrite it but keep the decoration. - if let Some(existing_value) = existing_value { - *new_value.decor_mut() = existing_value.decor().clone(); - *existing_value = new_value; - } else { - table.insert(requirement.name.as_ref(), Item::Value(new_value)); + // Try to find and update an existing entry, preserving its decoration. + let updated = self + .manifest_mut() + .get_or_insert_nested_table(&keys) + .map(|table| { + let existing_value = table.iter_mut().find_map(|(key, value)| { + let existing_name = pep508_rs::PackageName::from_str(key.get()).ok()?; + if existing_name == requirement.name { + value.as_value_mut() + } else { + None + } + }); + + if let Some(existing_value) = existing_value { + let mut cloned = new_value.clone(); + *cloned.decor_mut() = existing_value.decor().clone(); + *existing_value = cloned; + true + } else { + false + } + })?; + + if !updated { + self.manifest_mut().insert_into_inline_table( + &keys, + requirement.name.as_ref(), + new_value, + )?; } // Remove the entry from the project native array. @@ -498,26 +515,38 @@ impl ManifestDocument { .with_feature_name(Some(feature_name)) .with_table(Some(consts::PYPI_DEPENDENCIES)); - let table = self - .manifest_mut() - .get_or_insert_nested_table(&dependency_table_name.as_keys())?; - - let mut new_value = Value::from(pypi_requirement); - - let existing_value = table.iter_mut().find_map(|(key, value)| { - let existing_name = pep508_rs::PackageName::from_str(key.get()).ok()?; - if existing_name == requirement.name { - value.as_value_mut() - } else { - None - } - }); + let keys = dependency_table_name.as_keys(); + let new_value = Value::from(pypi_requirement); - if let Some(existing_value) = existing_value { - *new_value.decor_mut() = existing_value.decor().clone(); - *existing_value = new_value; - } else { - table.insert(requirement.name.as_ref(), Item::Value(new_value)); + let updated = self + .manifest_mut() + .get_or_insert_nested_table(&keys) + .map(|table| { + let existing_value = table.iter_mut().find_map(|(key, value)| { + let existing_name = pep508_rs::PackageName::from_str(key.get()).ok()?; + if existing_name == requirement.name { + value.as_value_mut() + } else { + None + } + }); + + if let Some(existing_value) = existing_value { + let mut cloned = new_value.clone(); + *cloned.decor_mut() = existing_value.decor().clone(); + *existing_value = cloned; + true + } else { + false + } + })?; + + if !updated { + self.manifest_mut().insert_into_inline_table( + &keys, + requirement.name.as_ref(), + new_value, + )?; } } else if feature_name.is_default() || matches!(location, Some(PypiDependencyLocation::Dependencies)) @@ -1210,4 +1239,175 @@ platforms = [] insta::assert_snapshot!(result); } + + /// Adding a new dependency to a multiline inline table should preserve + /// the multiline layout and trailing comma style. + #[test] + pub fn add_dependency_preserves_multiline_inline_table() { + let manifest_content = r#"[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.test] +dependencies = { + numpy = "*", + scipy = ">=1.0", +} +"#; + + let mut document = ManifestDocument::PixiToml(TomlDocument::new( + DocumentMut::from_str(manifest_content).unwrap(), + )); + + document + .add_dependency( + &PackageName::from_str("pandas").unwrap(), + &PixiSpec::Version(">=2.0".parse().unwrap()), + SpecType::Run, + None, + &FeatureName::from_str("test").unwrap(), + ) + .unwrap(); + + insta::assert_snapshot!(document.to_string()); + } + + /// Same as above, but the table has no trailing comma — the new entry + /// should also omit the trailing comma. + #[test] + pub fn add_dependency_preserves_multiline_inline_table_no_trailing_comma() { + let manifest_content = r#"[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.test] +dependencies = { + numpy = "*", + scipy = ">=1.0" +} +"#; + + let mut document = ManifestDocument::PixiToml(TomlDocument::new( + DocumentMut::from_str(manifest_content).unwrap(), + )); + + document + .add_dependency( + &PackageName::from_str("pandas").unwrap(), + &PixiSpec::Version(">=2.0".parse().unwrap()), + SpecType::Run, + None, + &FeatureName::from_str("test").unwrap(), + ) + .unwrap(); + + insta::assert_snapshot!(document.to_string()); + } + + /// Updating an existing dependency inside a multiline inline table should + /// preserve the table's layout and the entry's decoration. + #[test] + pub fn add_dependency_update_in_multiline_inline_table() { + let manifest_content = r#"[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.test] +dependencies = { + numpy = "*", + scipy = ">=1.0", +} +"#; + + let mut document = ManifestDocument::PixiToml(TomlDocument::new( + DocumentMut::from_str(manifest_content).unwrap(), + )); + + document + .add_dependency( + &PackageName::from_str("numpy").unwrap(), + &PixiSpec::Version(">=2.0".parse().unwrap()), + SpecType::Run, + None, + &FeatureName::from_str("test").unwrap(), + ) + .unwrap(); + + insta::assert_snapshot!(document.to_string()); + } + + /// Adding a new pypi dependency to a multiline inline table should + /// preserve the multiline layout. + #[test] + pub fn add_pypi_dependency_preserves_multiline_inline_table() { + let manifest_content = r#"[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.ml] +pypi-dependencies = { + numpy = ">=1.20.0", + scipy = ">=1.0", +} +"#; + + let mut document = ManifestDocument::PixiToml(TomlDocument::new( + DocumentMut::from_str(manifest_content).unwrap(), + )); + + let req = pep508_rs::Requirement::from_str("pandas>=2.0").unwrap(); + document + .add_pypi_dependency( + &req, + None, + None, + &FeatureName::from_str("ml").unwrap(), + None, + None, + ) + .unwrap(); + + insta::assert_snapshot!(document.to_string()); + } + + /// Adding a pypi dependency via the reuse-existing-feature-table path + /// in pyproject.toml should also preserve multiline inline table format. + #[test] + pub fn add_pypi_dependency_reuse_feature_preserves_multiline() { + let manifest_content = r#"[project] +name = "test" + +[tool.pixi.workspace] +channels = [] +platforms = [] + +[tool.pixi.feature.cuda] +pypi-dependencies = { + torch = ">=2.0.0", + jax = ">=0.4", +} +"#; + + let mut document = ManifestDocument::PyProjectToml(TomlDocument::new( + DocumentMut::from_str(manifest_content).unwrap(), + )); + + let numpy_req = pep508_rs::Requirement::from_str("numpy>=1.20.0").unwrap(); + document + .add_pypi_dependency( + &numpy_req, + None, + None, + &FeatureName::from_str("cuda").unwrap(), + None, + None, + ) + .unwrap(); + + insta::assert_snapshot!(document.to_string()); + } } diff --git a/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table.snap b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table.snap new file mode 100644 index 0000000000..f993c0d703 --- /dev/null +++ b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table.snap @@ -0,0 +1,15 @@ +--- +source: crates/pixi_manifest/src/manifests/document.rs +expression: document.to_string() +--- +[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.test] +dependencies = { + numpy = "*", + scipy = ">=1.0", + pandas = ">=2.0", +} diff --git a/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table_no_trailing_comma.snap b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table_no_trailing_comma.snap new file mode 100644 index 0000000000..40c59f0548 --- /dev/null +++ b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_preserves_multiline_inline_table_no_trailing_comma.snap @@ -0,0 +1,15 @@ +--- +source: crates/pixi_manifest/src/manifests/document.rs +expression: document.to_string() +--- +[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.test] +dependencies = { + numpy = "*", + scipy = ">=1.0", + pandas = ">=2.0" +} diff --git a/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_update_in_multiline_inline_table.snap b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_update_in_multiline_inline_table.snap new file mode 100644 index 0000000000..4079fed3cc --- /dev/null +++ b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_update_in_multiline_inline_table.snap @@ -0,0 +1,14 @@ +--- +source: crates/pixi_manifest/src/manifests/document.rs +expression: document.to_string() +--- +[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.test] +dependencies = { + numpy = ">=2.0", + scipy = ">=1.0", +} diff --git a/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_preserves_multiline_inline_table.snap b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_preserves_multiline_inline_table.snap new file mode 100644 index 0000000000..b02ba4768e --- /dev/null +++ b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_preserves_multiline_inline_table.snap @@ -0,0 +1,15 @@ +--- +source: crates/pixi_manifest/src/manifests/document.rs +expression: document.to_string() +--- +[workspace] +name = "test" +channels = [] +platforms = [] + +[feature.ml] +pypi-dependencies = { + numpy = ">=1.20.0", + scipy = ">=1.0", + pandas = ">=2.0", +} diff --git a/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_reuse_feature_preserves_multiline.snap b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_reuse_feature_preserves_multiline.snap new file mode 100644 index 0000000000..49348eabb2 --- /dev/null +++ b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_reuse_feature_preserves_multiline.snap @@ -0,0 +1,17 @@ +--- +source: crates/pixi_manifest/src/manifests/document.rs +expression: document.to_string() +--- +[project] +name = "test" + +[tool.pixi.workspace] +channels = [] +platforms = [] + +[tool.pixi.feature.cuda] +pypi-dependencies = { + torch = ">=2.0.0", + jax = ">=0.4", + numpy = ">=1.20.0", +}