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

fix(UsageText): consistent indent for help UsageText output #1279

Merged
merged 1 commit into from Jun 3, 2021

Conversation

clok
Copy link
Contributor

@clok clok commented Jun 2, 2021

What type of PR is this?

  • bug
  • cleanup
  • documentation
  • feature

What this PR does / why we need it:

This PR addresses the bug found by @codefromthecrypt and noted in #1278

Which issue(s) this PR fixes:

Fixes #1278

Testing

  • Added multiple unit tests that test the functionality for multi-line UsageText values for App, Command and SubCommand types.
  • Spot tested with a local CLI tool integration.

Release Notes

fix(UsageText): consistent indent for help UsageText output

Signed-off-by: Derek Smith <dsmith@goodwaygroup.com>
@clok clok force-pushed the fix/consistent-usagetext-indenting branch from 3646146 to 71379de Compare June 2, 2021 18:11
Copy link

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lovely!

@codefromthecrypt
Copy link

only thought is might want to also backfill a test that MarkdownDocTemplate does not indent any of the lines (maybe overkill..)

@clok
Copy link
Contributor Author

clok commented Jun 3, 2021

only thought is might want to also backfill a test that MarkdownDocTemplate does not indent any of the lines (maybe overkill..)

The markdown is a bit different. The values passed into the template are manipulated before the execution of the template render. Note the prepareCommands on line 59 as an example. https://github.com/urfave/cli/blob/master/docs.go#L56-L62

As a result, there shouldn't be a need to adjust the template or test for the MD generation. A part of me wants to refactor the MD generation to use template functions more consistently, but that is an out of scope effort that I think warrants more conversation before implementing.

@codefromthecrypt
Copy link

out of scope comment so hope it isn't too distracting. I found indeed markdown could be better handled differently, though to each their own on this. Particularly, there's formatting bias towards help output which doesn't work so well in markdown. I ended up using this which means I don't personally need the markdown feature at the moment:

	cli.MarkdownDocTemplate = `# GetEnvoy CLI Overview
{{ .App.UsageText }}

# Commands

| Name | Usage |
| ---- | ----- |
{{range $index, $cmd := .App.VisibleCommands}}{{if $index}}
{{end}}| {{$cmd.Name}} | {{$cmd.Usage}} |{{end}}
| --version, -v | Print the version of GetEnvoy |

# Environment Variables

| Name | Usage | Default |
| ---- | ----- | ------- |
{{range $index, $option := .App.VisibleFlags}}{{if $index}}
{{end}}| {{index $option.EnvVars 0}} | {{$option.Usage}} | {{$option.DefaultText}} |{{end}}
`

Copy link

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes the issue and markdown is out-of-scope!

@rliebz rliebz merged commit 443c6a5 into urfave:master Jun 3, 2021
@clok clok deleted the fix/consistent-usagetext-indenting branch June 4, 2021 03:57
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.

v2 bug: multi-line UsageText doesn't consistently indent in help output
3 participants