New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pkgs.lib.encodeGNUCommandLine
#75539
Conversation
This adds a new utility to intelligently convert Nix records to command line options to reduce boilerplate for simple use cases and to also reduce the likelihood of malformed command lines
I'm hesitant about this because it puts a highly specific policy of how command line arguments are encoded in the standard library. It's not at all clear how universally applicable this is. For example, The name Escaping shell arguments should be handled elsewhere since you could also pass arguments as an array. So escaping could be done separately (e.g. |
@edolstra: I would be open to naming this in such a way that it does not imply that it is the only way to render command-line options and/or making the utility's behavior customizable (e.g. customizing how it handles lists or boolean-valued fields). For example, conceptually this utility could take an additional record argument specifying how to handle each behavior (with default values so that the user can supply encodeGNUCommandLine =
{ renderBool ? key: value: if value then " ${hyphenate key}" else ""
, renderList ? key: values: ...
, renderOption ? key: value: ...
, ...
}
options: ... |
@edolstra: Alright, I believe this is ready for re-review now. I incorporated your feedback |
... as suggested by @roberth This also caught a bug in rendering lists, which this change also fixes
@roberth: I believe I addressed your feedback and this is ready for another review |
Where would this be used seeing it returns string? We are moving towards |
@jtojnar: A { config, lib, pkgs, ... }:
let
cfg = config.services.example;
options = { inherit (cfg) host port; };
in
{ options.services.example = {
enable = lib.mkEnableOption "example";
host = lib.mkOption { ... };
port = lib.mkOption { ... };
};
config.systemd.services.example = lib.mkIf cfg.enable {
wantedBy = [ "multi-user.target" ];
script = ''
${pkgs}/bin/example${lib.toGNUCommandLine options}
'';
};
} |
You could extract a function
This will require a preceding space as in |
Well, the function returns a string. To be useful for structured attrs, it would have to return a list. |
precisely |
... as suggested by @roberth
... as suggested by @roberth Co-Authored-By: Robert Hensing <roberth@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, this could be used to provide something like NixOS/rfcs#42 for CLI configs.
Just seeing this, it would have been good if it had followed the same interface as Not entirely sure what the separation in |
Oh, I’d also rename the functions to use |
Ah, no, the string is explicitely POSIX shell-formatted, so we should call it that. |
Let’s also not expose it into the global namespace, similar to |
@Profpatsch feel free to make a PR for the rename/move.
It's for overriding the convention, for example to allow Re POSIX: it does seem to be GNU, at least according to GNU.
-- https://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html I'd welcome a more generic term than GNU, but I don't know of any that implies long options. |
I created a PR with my suggested breaking changes: #78337 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Motivation for this change
This adds a new utility to intelligently convert Nix records to
command line options to reduce boilerplate for simple use cases and to
also reduce the likelihood of malformed command lines
Things done
I ran
nix-instantiate --eval --strict lib/tests/misc.nix