Skip to content
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

[fud2] Add subcommand to edit configuration #1882

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Conversation

rachitnigam
Copy link
Contributor

@rachitnigam rachitnigam commented Jan 29, 2024

Similar to fud c --edit but does not support querying keys yet. Tagging @sampsyo for visibility.

@rachitnigam rachitnigam enabled auto-merge (squash) January 29, 2024 02:33
@rachitnigam rachitnigam merged commit d6d5849 into main Jan 29, 2024
7 checks passed
@rachitnigam rachitnigam deleted the fud2-edit-conf branch January 29, 2024 02:44
@sampsyo
Copy link
Contributor

sampsyo commented Jan 29, 2024

Seems useful! One thing UI to chat about here: so far, I hadn't added subcommands to the CLI. The main goal was to avoid the need for the e we in practice need to use for every OG fud invocation. I hadn't really thought of optional subcommands, but it makes sense. I guess I would have done something like fud2 --edit-config or something if I had done it, but I don't object to fud2 edit-config either. (It doesn't seem to add any ambiguity, because fud2 edit-config isn't a reasonable build command even if you have a file called edit-config for whatever reason, because we don't know the output state.)

@rachitnigam
Copy link
Contributor Author

Yeah, I think getting rid of e is a good idea. I was implicitly going with the philosophy that "management options" like editing the config or something like fud check can be commands while other things are not.

@sampsyo
Copy link
Contributor

sampsyo commented Jan 29, 2024

Sounds legit. After all, for stuff like that, all the other CLI options (e.g., --from, -o) are irrelevant, so it would be a weird lie to make them "siblings" to those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants