lib/cli: add wrappingValueString option to toGNUCommandLineShell
#373332
Conversation
Use `lib.concatStrings` for readability, and move `optionValueSeparator` to the top.
llakala
force-pushed
the
gnuCliQuotedArgs
branch
from
ada3f25 to
8815b56
Compare
|
Fixed the failing |
llakala
changed the title
toGNUCommandLineShell
|
Yep, it's an OOM error (according to Mic92/nixpkgs-review#387). If someone with more RAM could run Working on fixing the other lib test right now. |
|
Ah, I see the trouble now. The code for |
The function currently doesn't provide a way to create a result like `--foo "bar"` or `--foo="bar"`, where the argument is quoted. This option resolves that. We set it to an empty string by default, so we can always interpolate it into the string, even if the user didn't set it.
llakala
force-pushed
the
gnuCliQuotedArgs
branch
from
8815b56 to
3e03bcb
Compare
It wasn't clear to me why `toGNUCommandLine` would take an input like this `foo = "bar"`, and turn it into `[ "foo" "bar" ]`. I implemented a change to this, only to realize that this behavior is actually for `toGNUCommandLineShell`. It uses `lib.escapeShellArgs`, which treats the input `[ "--foo" "bar" ]` and `[ "--foo bar" ]` differently. The comment will hopefully prevent others from going down the same rabbit hole that I did.
There was a problem hiding this comment.
This doesn't sound right, the point of toGNUCommandLineShell is to return a list of arguments, which you can then use with e.g. escapeShellArgs (or some different escaping mechanism) to turn it into a string suitable to pass to e.g. bash. Is there a reason this doesn't work for you? #373291 doesn't mention anything about that.
Well, it's And I think you may be confused because I made changes since the original post, as mentioned in the comments(should've made an edit clarifying, that's my bad). The plan was never to make it not return a list - it was to make it put an option and its value in the same list element. So, this input: {
foo = "bar";
hello = "hi";
}
when passed into `GNUcommandLineShell`, resulted in:
```nix
[
"--foo"
"bar"
"--hello"
"hi"
]My change would've created an output like this: [
"--foo bar"
"--hello hi"
]But I realized that would create unforeseen changes and break a lot of workflows, including |
Right, though both of those functions get the new I guess I'll rephrase my question: What's your use case for needing |
Closes #373291.
The function currently doesn't provide a way to create a result like
--foo "bar"or--foo="bar", where the argument is quoted. This new option resolves that. We set it to an empty string by default, so we can always interpolate it into the string, even if the user didn't set it.Included in this PR is also a very small change to the original behavior. Now, the input
foo = "bar"withoptionValueSeparatorunset will be transformed into[ "--foo bar" ], instead of[ "--foo" "bar" ]. This resulted in cleaner code, and I think it's more intuitive anyways.nixpkgs-reviewis running right now on my system as I write this. There could be some packages that rely on this buggy behavior as mentioned above. I'll add the results in a comment when it finishes.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.