feat(docs): generate cli docs directly from the struct

[akaladarshi]

Related Issues

Fixes: #12706

[rvagg]

you're going to need to replicate this too, but it should be pretty easy, the lotus config default is fairly simple:

lotus/cmd/lotus/config.go

Lines 32 to 40 in 07f2f69

	c := config.DefaultFullNode()
	cb, err := config.ConfigUpdate(c, nil, config.Commented(!cctx.Bool("no-comment")), config.DefaultKeepUncommented())
	if err != nil {
	return err
	}
	fmt.Println(string(cb))

[rvagg]

Looks like a lot of the logic for this lives in https://github.com/urfave/cli/blob/main/help.go and the templates in here (unfortunately not exported!) are here: https://github.com/urfave/cli/blob/main/template.go

Use this as an exercise in minimising the diff, that's your challenge - how can you get to near zero red/green lines in the diff here for the documentation output, aiming to only make changes where the existing script is getting it wrong (mostly missing some things I believe).

[BigLep]

@akaladarshi : I assume you'll re-request review when you're ready for it to be looked at again.

[rvagg]

--storagerepo should be --storagerepo value? and then the spacing for this longest line should be the same as the others

[rvagg]

do we have a minimum width setting here that needs adjusting? looks like we have 4 extra spaces than we need

[rvagg]

ah, this one is missing because Hidden: true - better build that into your code too, if a flag is Hidden, then exclude it from the list

[rvagg]

you need to obey this so it doesn't show up in docs, same as flags, check for Hidden and skip if true

[rvagg]

this is new because it's Hidden - because it's both deprecated and pretty unsafe to invoke so we don't want to encourage it

[rvagg]

Very close @akaladarshi, good work so far. I haven't done an in-depth review of the actual code, I'm mainly looking at the output and assuming that the code is good, I'll look over that next.

[rvagg]

why do these need to become functions?

[akaladarshi]

Previously, lotus and lotus-miner shared global command variables, causing conflicts when running app.Run("lotus-miner", "-h"). This led to jumbled help output like lotus-miner lotus auth.

[rvagg]

See below, I went for a bit of a trek to find out why this is the case and came up with an easier approach than rewriting all of these.

[rvagg]

Suggested change

	formatted = fmt.Sprintf(" %-30s %s (default: %s)", names, usage, value)
	defpfx := " "
	if usage == "" {
	defpfx = ""
	}
	formatted = fmt.Sprintf(" %-30s %s%s(default: %s)", names, usage, defpfx, value)

something like this would deal with the case where there's no usage string; alternatively you could extend usage by one space if it's not "".
I'm not sure this is strictly needed when you account for the Hidden flags but it would fix current alignment problems.

[rvagg]

Also this -30 is getting in the way when the names is too long. Like when --miner-repo value, --storagerepo comes in to play which is longer than you want.

Here's what I think I'd do given your current code structure so as not to have to make too many changes: make the this number a property of FlagFormatter that you pass in when you construct it (or make a default value you can override if you like) (you'll probably need to fmt.Sprintf the format itself to insert it). Then you do two passes iterating through your flags - the first pass looks at the maximum length of the names from GetData and then the second pass sets the maximum length if it's greater than 30 and does the actual print.

[akaladarshi]

@rvagg I think I found a minimum solution for this we can directly use the urface cli package for generating, Initially I though we need to print everything (hidden), but this will directly print the correct format value.

     customAppData := func() map[string]interface{} {
		return map[string]interface{}{
			"ExtraInfo": g.app.ExtraInfo,
		}
	}

	cli.HelpPrinterCustom(header, cli.AppHelpTemplate, g.app, customAppData())

[akaladarshi]

@rvagg I think I found a minimum solution for this we can directly use the urface cli package for generating, Initially I though we need to print everything (hidden), but this will directly print the correct format value.

     customAppData := func() map[string]interface{} {
		return map[string]interface{}{
			"ExtraInfo": g.app.ExtraInfo,
		}
	}

	cli.HelpPrinterCustom(header, cli.AppHelpTemplate, g.app, customAppData())

Removed the above change in favour of app.Run(args[]string), because it was not printing the default values of flags. Anyway app.Run internally uses cli.HelpPrinter to print the commands.

[rvagg]

@akaladarshi if you want to take a shortcut on this task, we could scope it just to doing what you've done so far - generate the docs from the structs, but leave the existing workflow that requires building the binaries in place, i.e. restore this Makefile target to the way it was. The remaining problem is the need to generate the default config files which currently need the binaries to do it, but we could defer that to a separate PR and get this one landed.

[rvagg]

Suggested change

	for name, app := range cliApps {
	generator := NewDocGenerator(outputDir, app)
	for name, app := range cliApps {
	// reset HelpName manually because shared Command objects get modified
	// by the first app they are added to
	for _, cmd := range app.Commands {
	cmd.HelpName = fmt.Sprintf("%s %s", app.HelpName, cmd.Name)
	}
	generator := NewDocGenerator(outputDir, app)

this is all that's needed to reset the problem that you're working around by making the Commands in functions: https://github.com/urfave/cli/blob/v2-maint/app.go#L232-L240

Unfortunately you can't just reset HelpName and re-run app.Setup() because it's a strictly run-once function, which is pretty annoying. But since we don't do anything sophisticated in HelpName I think it's good enough to just rewrite it here. Then you can undo all of the functions you had to make (sorry), but hopefully it's just a case of checking thise files out from master again.

[rvagg]

actually probably wont' be that easy because you had to put them in a new package, but hopefully it's straightforward enough to just un-func those shared cmds