lib: improve cli module #78337
lib: improve cli module #78337
Conversation
ofborg
bot
added
6.topic: policy discussion
10.rebuild-darwin: 0
10.rebuild-linux: 0
labels
| # how to string-format the option name; | ||
| # by default one character is a short option (`-`), | ||
| # more than one characters a long option (`--`). | ||
| mkOptionName ? |
There was a problem hiding this comment.
Good idea to avoid the last use of render here, but mk seems a bit odd. The mk prefix is normally used for functions that assemble attrsets or derivations. These functions deconstruct rather than construct, so I'd go with from.
There was a problem hiding this comment.
This whole exercise is to make it have the same interface that is used in generators.nix, which uses mk for conversion attributes.
There was a problem hiding this comment.
I see your good intent, but that module also seems to use mk for no apparent reason. It is not making instances of the term it prefixes. It is consuming them.
There was a problem hiding this comment.
yeah, it might not have been the best naming, but I’m arguing for consistency. We can’t change the generators.nix interface anymore, so we need to change this if we want consistency.
There was a problem hiding this comment.
And internal consistency > perfect naming, always.
There was a problem hiding this comment.
It's not clear how consistency necessitates choosing a misleading name. You could always add new utilities to generators.nix with a better naming convention later and deprecate the misleading naming convention without removing it.
There was a problem hiding this comment.
yeah, but for now let’s keep it that way.
lib/cli is very similar to generators, so it should follow largely the same interface. Similar to how generators isn’t exported, we should also namespace cli by default (plus “cli” is only three characters to type).
The semantic difference between `encode` and `to` is not apparent. Users are likely to confuse both functions (which leads to unexpected error messages about the wrong types). Like in `generators.nix`, all functions should be prefixed by `to`. Furthermore, converting to a string depends on the target context. In this case, it’s a POSIX shell, so we should name it that (compare `escapeShellArg` in `strings.nix`). We can later add versions that escape for embedding in e.g. python scripts or similar.
Mirrors the naming scheme in `generators.nix`, for consistency. Also rename `key` to `k` and value to `v` to aid readability to the code structure.
They are cut off after a few decimal places; we cannot in good faith define a default string representation with that.
Let’s call them by what they are, option names. `generators.mkValueStringDefault` is a better value string renderer than plain `toString`. Also add docs to all options.
Profpatsch
force-pushed
the
lib-improve-cli-module
branch
from
2cd18c3
to
7228a3c
Compare
|
I’ll merge, if we decide we want to rename and deprecate later, we can do that. |
lib: improve cli module (cherry picked from commit 07eb21c)
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Here are the changes I suggested in the original PR after it was merged a few days ago.
Since this module is so young, I think we can still make breaking changes to clean up the interface and make it conform more to existing functions. In particular, it’s extremely similar to how I structured
generators.nix, so I tried making it conform more to that.cc @Gabriel439 as original author.