refactor: feature names inside double quotes if containing a .

[mrswastik-robot]

fixes #3171

Code refactored is in table_name.rs:

if feature_str.contains('.') {
    parts.push(format!("\"{}\"", feature_str).leak());
} else {
    parts.push(feature_str);
}

Had to use leak() otherwise I was facing lifetime error.
Also added a test case, it's passing.

[ruben-arts]

It doesn't work with pixi add -f test.test bat as that will result in:

[feature.'"test'.'test"'.dependencies]
bat = "*"

[mrswastik-robot]

It doesn't work with pixi add -f test.test bat as that will result in:

[feature.'"test'.'test"'.dependencies]
bat = "*"

sorry, @ruben-arts, my bad! is it because the quotes are being included in the string rather than being treated as delimiters? Using format!("\"{}\"", feature_str) inserts literal quotes into the string, but TOML treats them differently...
I guess the quoting is being handled by the toml_edit library

[baszalmstra]

Maybe for simplicity and reuse we could just have function or struct to escape a string? I assume there are more reasons to add quotes to keys?

[mrswastik-robot]

Maybe for simplicity and reuse we could just have function or struct to escape a string? I assume there are more reasons to add quotes to keys?

Maybe for simplicity and reuse we could just have function or struct to escape a string? I assume there are more reasons to add quotes to keys?

Maybe for simplicity and reuse we could just have function or struct to escape a string? I assume there are more reasons to add quotes to keys?

yup I get the point, so should I write a generalised function like escape_toml_keys perhaps in a file like crates/pixi_manifest/src/utils/toml_utils.rs, that should work for characters that require esaping in TOML keys?

[baszalmstra]

Makes sense!

[ruben-arts]

pixi add -f test.test bat

Still results in:

[feature.'"test'.'test"'.dependencies]
bat = "*"

It would be good to add a simple integration test to figure out that the manifest can still be read after the pixi add command.

[nichmor]

hey @mrswastik-robot !

I think it would be cool to have an integration test that will verify if we do pixi add test.test it can still be parsed when using pixi run

You can take a look at the bootstrapping of the test project in test_main_cli.py from tests/integration_python folder and combine it with running the task test from test_run_cli.py

Lmk if you have any questions

[mrswastik-robot]

hi @nichmor @ruben-arts, I think I found the issue here. Integration test revealed that the current solution still results in [feature.'"test'.'test"'.dependencies], and ig that's mostly because of the manual escaping I have been doing through the escape_toml_key() function inside toml_utils.rs. The problem is that the function is getting applied too early in the pipeline, here's what happening:

// In table_name.rs
if !feature_name.is_default() {
    parts.push("feature");
    let feature_str = feature_name.as_str();
    let escaped_feature = escape_toml_key(feature_str); // ← Applied escaping here
    parts.push(escaped_feature); // ← Now contains "test.test" 
}

// Later in the pipeline, when creating TOML:
let table_name = parts.join("."); // ← "feature.\"test.test\".dependencies"
// Then split again by dots for TOML library:
let parts: Vec<&str> = table_name.split('.').collect(); 
// Result: ["feature", "\"test", "test\"", "dependencies"] ← thus broken result here

so, this results in double escaping...ig the solution is to let the TOML library handle the escaping by keeping parts separated, an example would be like this:

// WRONG: Escaping too early, then splitting later
"feature.\"test.test\".dependencies" → split('.') → ["feature", "\"test", "test\"", "dependencies"]

// RIGHT: Keep parts separate, let TOML library escape
["feature", "test.test", "dependencies"] → TOML library → [feature."test.test".dependencies]

Would love to hear your feedback on this.

[baszalmstra]

Letting the TOML library handle it sounds great!

[nichmor]

hey @mrswastik-robot ! I checked out your branch, and if I do cargo run -- add -f test.test rich in examples/pypi/pixi.toml

and then

graf@Nichitas-MacBook-Pro pypi % cargo run -- run python
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.99s
     Running `/Users/graf/projects/oss/pixi-build-branch/target/debug/pixi run python`
Error:   × Unexpected keys, expected only 'platforms', 'channels', 'channel-priority', 'target', 'dependencies', 'host-dependencies', 'build-
  │ dependencies', 'pypi-dependencies', 'activation', 'tasks', 'pypi-options', 'system-requirements'
    ╭─[/Users/graf/projects/oss/pixi-build-branch/examples/pypi/pixi.toml:38:19]
 37 │ 
 38 │ [feature.'"test'.'test"'.dependencies]
    ·                   ──┬──
    ·                     ╰── 'test"' was not expected here
 39 │ rich = "*"
    ╰────

I still have the error. Is it expected?

[mrswastik-robot]

hey @mrswastik-robot ! I checked out your branch, and if I do cargo run -- add -f test.test rich in examples/pypi/pixi.toml

and then

graf@Nichitas-MacBook-Pro pypi % cargo run -- run python
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.99s
     Running `/Users/graf/projects/oss/pixi-build-branch/target/debug/pixi run python`
Error:   × Unexpected keys, expected only 'platforms', 'channels', 'channel-priority', 'target', 'dependencies', 'host-dependencies', 'build-
  │ dependencies', 'pypi-dependencies', 'activation', 'tasks', 'pypi-options', 'system-requirements'
    ╭─[/Users/graf/projects/oss/pixi-build-branch/examples/pypi/pixi.toml:38:19]
 37 │ 
 38 │ [feature.'"test'.'test"'.dependencies]
    ·                   ──┬──
    ·                     ╰── 'test"' was not expected here
 39 │ rich = "*"
    ╰────

I still have the error. Is it expected?

yes, this was expected, I haven't pushed the solution yet, I will push it as soon as it's ready...the approach I am following is to let the TOML library handle the escaping