Repository: urfave/cli
Pull request #1119 (2020-04-28 06:46:41)

Add word-wrap support, with wrap length provided by the user
## What type of PR is this?

- feature

## What this PR does / why we need it:

The help text is difficult to read when the lines are long. This patch attempts to wrap long lines, if the user provides a terminal width to wrap at.

## Which issue(s) this PR fixes:

Updated version of #1044.

Refers to #287.

## Testing

Added a unit test (which can be expanded).

## Release Notes

```release-note
The help text is now wrapped to fit the terminal window.
```


mostynb: How does this look?

* Is it OK to add a new field to `helpPrinter`?
* The template probably needs tweaking to support wrapping more fields, but I don't know offhand which other fields to test.
* This makes no attempt to break long words.
lynncyrin: I see you just force pushed, so I'm not sure what changed? Can you re-request my review when this is ready for eyes again, and please not force push until after the PR is ready to merge?
mostynb: > I see you just force pushed, so I'm not sure what changed? Can you re-request my review when this is ready for eyes again, and please not force push until after the PR is ready to merge?

Sorry about that- I squashed the commit in an attempt to make it easy to verify that no new modules were added.

I have re-pushed the branch with [the original commit](https://github.com/urfave/cli/pull/1119/commits/be03ef5e9b4f9a8300e2e3ed5dd972c7840cbec0) followed by a [fixup that removes the newly added module](https://github.com/urfave/cli/pull/1119/commits/7c8032c545db2da51ab2ec4d85f567e5b71009ce).
lynncyrin: I'll try to give this a look soon!
ericmillin: @lynncyrin Would love to see this merged.
lynncyrin: Oops 😄
xordspar0: Is this ready to be merged?
mostynb: > Is this ready to be merged?

It's stalled, waiting for review.
mostynb: I was looking into the conflicting files, and noticed that #1175 has landed, which implements a subset of the feature here.

Force-pushed a squashed version of this with two fixes appended, which makes `TestWrappedHelp` also test user specifed indenting, and another change which makes `Test_ShowAppHelp_MultiLineDescription` on master pass if I revert the code changes from be9c0378066dc5baf84a56005f90317cca6ecd84 and then apply this PR.

How should we proceed?
rliebz: > I was looking into the conflicting files, and noticed that #1175 has landed, which implements a subset of the feature here.
> 
> Force-pushed a squashed version of this with two fixes appended, which makes `TestWrappedHelp` also test user specifed indenting, and another change which makes `Test_ShowAppHelp_MultiLineDescription` on master pass if I revert the code changes from [be9c037](https://github.com/urfave/cli/commit/be9c0378066dc5baf84a56005f90317cca6ecd84) and then apply this PR.
> 
> How should we proceed?

Those changes were contributed because someone needed them, and then tagged in a release. I think reverting them is off the table.

If you're still interested in getting this change in, you'll need to resolve conflicts. I'm totally fine if we change the template—I think folks using the default template are just signing up for whatever we decide makes sense. That said, the new template functions (indent, nindent, and trim) are now part of the public API, so those are something we shouldn't be removing.
mostynb: > Those changes were contributed because someone needed them, and then tagged in a release. I think reverting them is off the table.
> 
> If you're still interested in getting this change in, you'll need to resolve conflicts. I'm totally fine if we change the template—I think folks using the default template are just signing up for whatever we decide makes sense. That said, the new template functions (indent, nindent, and trim) are now part of the public API, so those are something we shouldn't be removing.

Pushed a rebased and squashed version, which undoes the template changes from be9c0378066dc5baf84a56005f90317cca6ecd84, but leaves the `indent`, `nindent` and `trim` functions in the function map for anyone who started using them with a custom template.
mostynb: Updated the template and added some tests to wrap commands and subcommands. There are probably other items that we can improve, but I think this is a good start.
mostynb: How does this look now?
stale[bot]: This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

mostynb: Ping.
stale[bot]: This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

stale[bot]: This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

stale[bot]: Closing this as it has become stale.

meatballhat: @mostynb Sorry for the delay!
meatballhat: @mostynb Are you up for resolving the conflicts and moving this work forward? No worries if not. I'm happy to pick it up ❤️ 
mostynb: I'll take a look and let you know if I run into trouble.
mostynb: @meatballhat: the failing test also fails for me locally on the tip of main. The CI failure is also visible in this typo fix PR: #1402
meatballhat: @mostynb this looks like the problem of the docs tests running against latest release instead of current working tree :weary:

bumping the priority of that work now...
meatballhat: @mostynb Attempted fix for the test-docs step at #1403 

Patch:
diff --git a/help.go b/help.go
index ff59ddc..9a8d243 100644
--- a/help.go
+++ b/help.go
@@ -64,6 +64,11 @@ var HelpPrinter helpPrinter = printHelp
 // HelpPrinterCustom is a function that writes the help output. It is used as
 // the default implementation of HelpPrinter, and may be called directly if
 // the ExtraInfo field is set on an App.
+//
+// In the default implementation, if the customFuncs argument contains a
+// "wrapAt" key, which is a function which takes no arguments and returns
+// an int, this int value will be used to produce a "wrap" function used
+// by the default template to wrap long lines.
 var HelpPrinterCustom helpPrinterCustom = printHelpCustom
 
 // VersionPrinter prints the version for the App
@@ -286,12 +291,29 @@ func ShowCommandCompletions(ctx *Context, command string) {
 // The customFuncs map will be combined with a default template.FuncMap to
 // allow using arbitrary functions in template rendering.
 func printHelpCustom(out io.Writer, templ string, data interface{}, customFuncs map[string]interface{}) {
+
+	const maxLineLength = 10000
+
 	funcMap := template.FuncMap{
 		"join":    strings.Join,
 		"indent":  indent,
 		"nindent": nindent,
 		"trim":    strings.TrimSpace,
+		"wrap":    func(input string, offset int) string { return wrap(input, offset, maxLineLength) },
+		"offset":  offset,
+	}
+
+	if customFuncs["wrapAt"] != nil {
+		if wa, ok := customFuncs["wrapAt"]; ok {
+			if waf, ok := wa.(func() int); ok {
+				wrapAt := waf()
+				customFuncs["wrap"] = func(input string, offset int) string {
+					return wrap(input, offset, wrapAt)
+				}
+			}
+		}
 	}
+
 	for key, value := range customFuncs {
 		funcMap[key] = value
 	}
@@ -402,3 +424,55 @@ func indent(spaces int, v string) string {
 func nindent(spaces int, v string) string {
 	return "\n" + indent(spaces, v)
 }
+
+func wrap(input string, offset int, wrapAt int) string {
+	var sb strings.Builder
+
+	lines := strings.Split(input, "\n")
+
+	padding := strings.Repeat(" ", offset)
+
+	for i, line := range lines {
+		if i != 0 {
+			sb.WriteString(padding)
+		}
+
+		sb.WriteString(wrapLine(line, offset, wrapAt, padding))
+
+		if i != len(lines)-1 {
+			sb.WriteString("\n")
+		}
+	}
+
+	return sb.String()
+}
+
+func wrapLine(input string, offset int, wrapAt int, padding string) string {
+	if wrapAt <= offset || len(input) <= wrapAt-offset {
+		return input
+	}
+
+	lineWidth := wrapAt - offset
+	words := strings.Fields(input)
+	if len(words) == 0 {
+		return input
+	}
+
+	wrapped := words[0]
+	spaceLeft := lineWidth - len(wrapped)
+	for _, word := range words[1:] {
+		if len(word)+1 > spaceLeft {
+			wrapped += "\n" + padding + word
+			spaceLeft = lineWidth - len(word)
+		} else {
+			wrapped += " " + word
+			spaceLeft -= 1 + len(word)
+		}
+	}
+
+	return wrapped
+}
+
+func offset(input string, fixed int) int {
+	return len(input) + fixed
+}
diff --git a/template.go b/template.go
index 264eb85..f3116fd 100644
--- a/template.go
+++ b/template.go
@@ -4,16 +4,16 @@ package cli
 // cli.go uses text/template to render templates. You can
 // render custom help text by setting this variable.
 var AppHelpTemplate = `NAME:
-   {{.Name}}{{if .Usage}} - {{.Usage}}{{end}}
+   {{$v := offset .Name 6}}{{wrap .Name 3}}{{if .Usage}} - {{wrap .Usage $v}}{{end}}
 
 USAGE:
-   {{if .UsageText}}{{.UsageText | nindent 3 | trim}}{{else}}{{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Version}}{{if not .HideVersion}}
+   {{if .UsageText}}{{wrap .UsageText 3}}{{else}}{{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Version}}{{if not .HideVersion}}
 
 VERSION:
    {{.Version}}{{end}}{{end}}{{if .Description}}
 
 DESCRIPTION:
-   {{.Description | nindent 3 | trim}}{{end}}{{if len .Authors}}
+   {{wrap .Description 3}}{{end}}{{if len .Authors}}
 
 AUTHOR{{with $length := len .Authors}}{{if ne 1 $length}}S{{end}}{{end}}:
    {{range $index, $author := .Authors}}{{if $index}}
@@ -31,26 +31,26 @@ GLOBAL OPTIONS:{{range .VisibleFlagCategories}}
 
 GLOBAL OPTIONS:
    {{range $index, $option := .VisibleFlags}}{{if $index}}
-   {{end}}{{$option}}{{end}}{{end}}{{end}}{{if .Copyright}}
+   {{end}}{{wrap $option.String 6}}{{end}}{{end}}{{end}}{{if .Copyright}}
 
 COPYRIGHT:
-   {{.Copyright}}{{end}}
+   {{wrap .Copyright 3}}{{end}}
 `
 
 // CommandHelpTemplate is the text template for the command help topic.
 // cli.go uses text/template to render templates. You can
 // render custom help text by setting this variable.
 var CommandHelpTemplate = `NAME:
-   {{.HelpName}} - {{.Usage}}
+   {{$v := offset .HelpName 6}}{{wrap .HelpName 3}}{{if .Usage}} - {{wrap .Usage $v}}{{end}}
 
 USAGE:
-   {{if .UsageText}}{{.UsageText | nindent 3 | trim}}{{else}}{{.HelpName}}{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Category}}
+   {{if .UsageText}}{{wrap .UsageText 3}}{{else}}{{.HelpName}}{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Category}}
 
 CATEGORY:
    {{.Category}}{{end}}{{if .Description}}
 
 DESCRIPTION:
-   {{.Description | nindent 3 | trim}}{{end}}{{if .VisibleFlagCategories}}
+   {{wrap .Description 3}}{{end}}{{if .VisibleFlagCategories}}
 
 OPTIONS:{{range .VisibleFlagCategories}}
    {{if .Name}}{{.Name}}
@@ -69,10 +69,10 @@ var SubcommandHelpTemplate = `NAME:
    {{.HelpName}} - {{.Usage}}
 
 USAGE:
-   {{if .UsageText}}{{.UsageText | nindent 3 | trim}}{{else}}{{.HelpName}} command{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Description}}
+   {{if .UsageText}}{{wrap .UsageText 3}}{{else}}{{.HelpName}} command{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Description}}
 
 DESCRIPTION:
-   {{.Description | nindent 3 | trim}}{{end}}
+   {{wrap .Description 3}}{{end}}
 
 COMMANDS:{{range .VisibleCategories}}{{if .Name}}
    {{.Name}}:{{range .VisibleCommands}}


Test patch:
diff --git a/help_test.go b/help_test.go
index 17a263d..4feb7f0 100644
--- a/help_test.go
+++ b/help_test.go
@@ -1124,3 +1124,225 @@ func TestDefaultCompleteWithFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestWrappedHelp(t *testing.T) {
+
+	// Reset HelpPrinter after this test.
+	defer func(old helpPrinter) {
+		HelpPrinter = old
+	}(HelpPrinter)
+
+	output := new(bytes.Buffer)
+	app := &App{
+		Writer: output,
+		Flags: []Flag{
+			&BoolFlag{Name: "foo",
+				Aliases: []string{"h"},
+				Usage:   "here's a really long help text line, let's see where it wraps. blah blah blah and so on.",
+			},
+		},
+		Usage:     "here's a sample App.Usage string long enough that it should be wrapped in this test",
+		UsageText: "i'm not sure how App.UsageText differs from App.Usage, but this should also be wrapped in this test",
+		// TODO: figure out how to make ArgsUsage appear in the help text, and test that
+		Description: `here's a sample App.Description string long enough that it should be wrapped in this test
+
+with a newline
+   and an indented line`,
+		Copyright: `Here's a sample copyright text string long enough that it should be wrapped.
+Including newlines.
+   And also indented lines.
+
+
+And then another long line. Blah blah blah does anybody ever read these things?`,
+	}
+
+	c := NewContext(app, nil, nil)
+
+	HelpPrinter = func(w io.Writer, templ string, data interface{}) {
+		funcMap := map[string]interface{}{
+			"wrapAt": func() int {
+				return 30
+			},
+		}
+
+		HelpPrinterCustom(w, templ, data, funcMap)
+	}
+
+	_ = ShowAppHelp(c)
+
+	expected := `NAME:
+    - here's a sample
+      App.Usage string long
+      enough that it should be
+      wrapped in this test
+
+USAGE:
+   i'm not sure how
+   App.UsageText differs from
+   App.Usage, but this should
+   also be wrapped in this
+   test
+
+DESCRIPTION:
+   here's a sample
+   App.Description string long
+   enough that it should be
+   wrapped in this test
+   
+   with a newline
+      and an indented line
+
+GLOBAL OPTIONS:
+   --foo, -h here's a
+      really long help text
+      line, let's see where it
+      wraps. blah blah blah
+      and so on. (default:
+      false)
+
+COPYRIGHT:
+   Here's a sample copyright
+   text string long enough
+   that it should be wrapped.
+   Including newlines.
+      And also indented lines.
+   
+   
+   And then another long line.
+   Blah blah blah does anybody
+   ever read these things?
+`
+
+	if output.String() != expected {
+		t.Errorf("Unexpected wrapping, got:\n%s\nexpected: %s",
+			output.String(), expected)
+	}
+}
+
+func TestWrappedCommandHelp(t *testing.T) {
+
+	// Reset HelpPrinter after this test.
+	defer func(old helpPrinter) {
+		HelpPrinter = old
+	}(HelpPrinter)
+
+	output := new(bytes.Buffer)
+	app := &App{
+		Writer: output,
+		Commands: []*Command{
+			{
+				Name:        "add",
+				Aliases:     []string{"a"},
+				Usage:       "add a task to the list",
+				UsageText:   "this is an even longer way of describing adding a task to the list",
+				Description: "and a description long enough to wrap in this test case",
+				Action: func(c *Context) error {
+					return nil
+				},
+			},
+		},
+	}
+
+	c := NewContext(app, nil, nil)
+
+	HelpPrinter = func(w io.Writer, templ string, data interface{}) {
+		funcMap := map[string]interface{}{
+			"wrapAt": func() int {
+				return 30
+			},
+		}
+
+		HelpPrinterCustom(w, templ, data, funcMap)
+	}
+
+	_ = ShowCommandHelp(c, "add")
+
+	expected := `NAME:
+    - add a task to the list
+
+USAGE:
+   this is an even longer way
+   of describing adding a task
+   to the list
+
+DESCRIPTION:
+   and a description long
+   enough to wrap in this test
+   case
+`
+
+	if output.String() != expected {
+		t.Errorf("Unexpected wrapping, got:\n%s\nexpected: %s",
+			output.String(), expected)
+	}
+}
+
+func TestWrappedSubcommandHelp(t *testing.T) {
+
+	// Reset HelpPrinter after this test.
+	defer func(old helpPrinter) {
+		HelpPrinter = old
+	}(HelpPrinter)
+
+	output := new(bytes.Buffer)
+	app := &App{
+		Name:   "cli.test",
+		Writer: output,
+		Commands: []*Command{
+			{
+				Name:        "bar",
+				Aliases:     []string{"a"},
+				Usage:       "add a task to the list",
+				UsageText:   "this is an even longer way of describing adding a task to the list",
+				Description: "and a description long enough to wrap in this test case",
+				Action: func(c *Context) error {
+					return nil
+				},
+				Subcommands: []*Command{
+					{
+						Name:      "grok",
+						Usage:     "remove an existing template",
+						UsageText: "longer usage text goes here, la la la, hopefully this is long enough to wrap even more",
+						Action: func(c *Context) error {
+							return nil
+						},
+					},
+				},
+			},
+		},
+	}
+
+	HelpPrinter = func(w io.Writer, templ string, data interface{}) {
+		funcMap := map[string]interface{}{
+			"wrapAt": func() int {
+				return 30
+			},
+		}
+
+		HelpPrinterCustom(w, templ, data, funcMap)
+	}
+
+	_ = app.Run([]string{"foo", "bar", "grok", "--help"})
+
+	expected := `NAME:
+   cli.test bar grok - remove
+                       an
+                       existing
+                       template
+
+USAGE:
+   longer usage text goes
+   here, la la la, hopefully
+   this is long enough to wrap
+   even more
+
+OPTIONS:
+   --help, -h  show help (default: false)
+   
+`
+
+	if output.String() != expected {
+		t.Errorf("Unexpected wrapping, got:\n%s\nexpected: %s",
+			output.String(), expected)
+	}
+}