caddy
caddy copied to clipboard
caddyserver
cmd: migrate to github.com/spf13/cobra
This PR swaps the homegrown CLI parsing and management with github.com/muesli/coral, a fork of github.com/spf13/cobra. The switch brings the benefit of being able to generate shell-completions for major shells (PowerShell, bash, zsh, fish) and manpages. It's a POC/proposal just to see the impact of the change. It brings only 1 direct dependency and 3 transitive ones.
The current tricky part is figuring out how to adapt the current caddycmd.Command to *coral.Command. The challenge is in mapping the flags. Once that is figured out, all is easy.
P.S.: Never mind the deletions in commandfunc.go. I'm only using that to track which ones aren't ported yet. Once all is ported, I'll split the funcs back into the file.
Edit: As of Cobra v1.4.0, there is not dependency on Viper, which thins its dependency to minimum and matches coral, so swapping coral for cobra does not introduce any new deps (across the two).
Supercedes caddyserver/dist#73
Worth noting this could be a breaking change for anyone who happens to use - single dash flags instead of -- double dash, because this uses pflags instead. But we've always documented -- everywhere so it shouldn't be a problem.
caddy help used to show the wrong way, and there was an issue opened about that, which we closed as declined at the time https://github.com/caddyserver/caddy/issues/4240
But this is also a benefit in the sense that we could now provide single letter shorthands for some flags.
The single-dash flags seem to be already used and referenced across the interwebs, examples:
- https://wiki.archlinux.org/title/Caddy
- https://mercure.rocks/docs/hub/install
- https://www.booleanworld.com/host-website-caddy-web-server-linux/
This PR introduces a breaking change to the CLI. Scripts and systemd units using the single-dash flag will break and stop working. There doesn't seem a way to do this in backwards compatible manner, unless I'm missing something.
Thanks for working on this Mohammed. It's been a while since I looked at the manpages thing; is the main advantage of this approach over https://github.com/caddyserver/dist/pull/73 the support for multiple shells?
Given the extensive changes needed and the single-hyphen thing, I am not sure if it is worth it at this point... which is a bummer to say, but... what do you think?
I'd love to get shortcuts for flags so we could do stuff like caddy adapt -c=/path/to/Caddyfile -p. Nice and short. Annoying to type out --pretty every time I'm playing around.
I personally think the breakage is worth it for the UX benefits. The help docs will be normalized nicely (no longer confusingly use -config form etc), and completions is a nice win.
This is also overall less code, because some stuff we did ourselves is handled by coral automatically already (like the help command).
But we do need to make it clear in the release notes when this goes out that this is a breaking change for those that do use the - form which we already did discourage (albeit softly). This won't break anyone's Docker or Systemd setups.
Thanks for working on this Mohammed. It's been a while since I looked at the manpages thing; is the main advantage of this approach over caddyserver/dist#73 the support for multiple shells?
The capability to generate, correct, and consistent shell auto-completion scripts is a pretty major gain, IMO. Francis and I worked on the auto-completion support (caddyserver/dist#33 and caddyserver/dist#35), and it's quite a churn to keep the completion scripts and the caddy commands/flags in sync. Note how we haven't updated it recently, and it's missing the upgrade, add-package, and remove-package commands.
Not to mention none of the available external tooling for generating shell-completions is perfect, each has its own quirks, and I had to make few compromises along the way when I surveyed the scene. Moreover, the current approach to generate the completion script does not accommodate commands or flags plugged by non-standard caddy modules. Coral will always generate consistent and complete shell-completion scripts.
I'm not a fan of the breakage, but the maintainability gains are a big plus. We need to communicate the breakage well enough for everyone to validate their scripts.
As of Cobra v1.4.0, there is not dependency on Viper, which thins its dependency to minimum and matches coral, so swapping coral for cobra does not introduce any new deps (across the two).
Cool, looking forward to checking this out once I am back in the office. Thanks Mohammed!
This is looking good. I like the smaller dep tree with Cobra 1.4 as well. Just going to pull it down and spin it around before I review it!
I just realized that single-hyphen flags were documented when using the standard Go -h flag to get help. So that might be a bummer. Probably not a showstopper, but a bit of a bummer.
Actually maybe it's fine, since our website docs use -- and those who are using caddy -h or similar are having the CLI itself give them help for the CLI. Unless they're using that to write scripts to automate it in production... sigh, I dunno.
We might need to merge this in after a big release that explicitly notes that - is deprecated in preparation for this change.