pixi
pixi copied to clipboard
refactor: feature names inside double quotes if containing a `.`
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.
It doesn't work with pixi add -f test.test bat as that will result in:
[feature.'"test'.'test"'.dependencies]
bat = "*"
It doesn't work with
pixi add -f test.test batas 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
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?
Makes sense!
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.
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.
Letting the TOML library handle it sounds great!
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?
hey @mrswastik-robot ! I checked out your branch, and if I do
cargo run -- add -f test.test richin examples/pypi/pixi.tomland 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
https://github.com/prefix-dev/pixi/issues/3171 has already been fixed