Add keygen command. #9
Conversation
This reverts commit 643ab86. See: <#9 (comment)>
src/commands/keygen.rs
Outdated
| public_key.algorithm().to_int(), | ||
| public_key.key_tag() | ||
| ); | ||
| // TODO: Adjust for how 'Env' mocks the current directory. |
There was a problem hiding this comment.
It's on main so you could do this (and make all this testable)
There was a problem hiding this comment.
Ah yeah, I need to rebase now.
| algorithm = parse_os_with("algorithm (-a)", &parser.value()?, |s| { | ||
| Ok(match s { | ||
| "list" => { | ||
| // TODO: Mock stdout and process exit? |
There was a problem hiding this comment.
This is possible after the notify branch is merged, where parse_ldns takes Env as well.
|
|
||
| Arg::Short('r') => { | ||
| // NOTE: '-r' can be repeated; we don't use it, so the order doesn't matter. | ||
| let _ = parser.value()?; |
There was a problem hiding this comment.
Do we want to warn or error here? Sounds like something that could lead to unexpected behaviour.
There was a problem hiding this comment.
Do you mean warning that we don't support -r at all? Perhaps that's warranted, yeah.
There was a problem hiding this comment.
Explicit error saying that it's not supported yeah.
| impl From<Keygen> for Command { | ||
| fn from(value: Keygen) -> Self { | ||
| Self::Keygen(value) | ||
| } | ||
| } |
There was a problem hiding this comment.
I put all of these in commands/mod.rs, though maybe you're right it should be here.
There was a problem hiding this comment.
Putting them here simplifies some of the merge conflicts we get for every new command.
There was a problem hiding this comment.
I'm mostly opposed to the inconsistency
|
|
||
| /// Create a symlink, overwriting if it already exists. | ||
| #[cfg(unix)] | ||
| fn fs_symlink_force(&self, target: impl AsRef<Path>, link: impl AsRef<Path>) -> Result<()> { |
There was a problem hiding this comment.
I'm not sure about adding higher-level API's than std here.
There was a problem hiding this comment.
For the purpose of CLI programs, std actually provides relatively little. In most languages, CLI codebases tend to have their own utility modules with these kinds of specialized filesystem / OS operations, often matching up with common command-line tools (e.g. symlink_force is basically ln -sf). I think we're going to accumulate these kinds of utility functions somewhere, and while Env isn't necessarily the right place, I think it's certainly the most convenient.
There was a problem hiding this comment.
I'm confused, you said yourself that adding a lot of functions to Env is probably not the right way to go? Besides, it depends on the function, I think we should keep them with the tool as long as they're not used in other tools.
This kind of muddles the idea of Env into both low-level environment interaction a util module. A function that takes Env as first argument is just as effective.
Don't consider this blocking though, it's alright.
WIP.