Add newline after flags

[aimichal]

What type of PR is this?

• bug

What this PR does / why we need it:

Adds missing newlines after flags in help output.

Which issue(s) this PR fixes:

Fixes #1514

Testing

I made the change locally and recompiled my app and saw the help output has newlines after each flag.

Release Notes

Newlines are added after each flag in help output.

[julian7]

Hmm, are there non-categorizable flags at all? I mean, all built-in types define GetCategory(), therefore the second "OPTIONS" will hardly ever be hit.

[julian7]

This line results an empty line with three spaces, and two empty lines if there are any options, which breaks tests.

I'm thinking of modifying this template like this:

 OPTIONS:{{range .VisibleFlagCategories}}
    {{if .Name}}{{.Name}}
-   {{end}}{{range .Flags}}{{.}}{{end}}{{end}}{{else}}{{if .VisibleFlags}}
+   {{end}}{{range .Flags}}{{.}}
+{{else}}
+{{end}}{{end}}{{else}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}{{end}}{{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}
+{{else}}
+{{end}}{{end}}{{end -}}
 `

This will get rid of the last empty line, and inserts one if there are no flags in the list.

[dearchap]

@julian7 Do you run make test after this change ? Since it impacts the unit tests those need to be updated as welll.

[julian7]

I did something (which is very similar, probably the same as here), and I ran go test on them until they came back 100% successful. Then, I just copied the git diff portion here :)

This is what I have now (EDIT: added test output):

js@hutton in cli ☸️  yolo on  main [!] via 🐹 v1.19.1 
❯ git show | cat
commit 8335f5435f27584829776f1a34289f0da9a0a0d2
Merge: 6491dde 15b2789
Author: dearchap <dear.chap@gmail.com>
Date:   Wed Oct 5 08:33:35 2022 -0400

    Merge pull request #1513 from abitrolly/patch-1
    
    wrap: Avoid trailing whitespace for empty lines

js@hutton in cli ☸️  yolo on  main [!] via 🐹 v1.19.1 
❯ go test | tail
   help, h  Shows a list of commands or help for one command

GLOBAL OPTIONS:
   --help, -h   show help (default: false)
   --opt value  
   
*cli.stringGeneric 1
*cli.stringGeneric 
PASS
ok      github.com/urfave/cli/v2        0.146s

js@hutton in cli ☸️  yolo on  main [!] via 🐹 v1.19.1
❯ git diff | cat
diff --git help_test.go help_test.go
index d7cdfa2..b0bdb7e 100644
--- help_test.go
+++ help_test.go
@@ -1370,7 +1370,7 @@ OPTIONS:
 `
 
        if output.String() != expected {
-               t.Errorf("Unexpected wrapping, got:\n%s\nexpected:\n%s",
+               t.Errorf("Unexpected wrapping, got:\n%q\nexpected:\n%q",
                        output.String(), expected)
        }
 }
@@ -1439,7 +1439,7 @@ OPTIONS:
 `
 
        if output.String() != expected {
-               t.Errorf("Unexpected wrapping, got:\n%s\nexpected: %s",
+               t.Errorf("Unexpected wrapping, got:\n%q\nexpected:\n%q",
                        output.String(), expected)
        }
 }
diff --git template.go template.go
index 9e13604..142b798 100644
--- template.go
+++ template.go
@@ -54,10 +54,14 @@ DESCRIPTION:
 
 OPTIONS:{{range .VisibleFlagCategories}}
    {{if .Name}}{{.Name}}
-   {{end}}{{range .Flags}}{{.}}{{end}}{{end}}{{else}}{{if .VisibleFlags}}
+   {{end}}{{range .Flags}}{{.}}
+{{else}}
+{{end}}{{end}}{{else}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}{{end}}{{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}
+{{else}}
+{{end}}{{end}}{{end -}}
 `
 
 // SubcommandHelpTemplate is the text template for the subcommand help topic.

[aimichal]

@julian7 Thanks for asking... But I don't know what "non-categorizable flags" means :-)

I will come up with an isolated example to demonstrate change. I also noticed some of the tests failed in CI. I'll take a look and try to update them or come up with a test to exercise this.

(If this fix is urgent for others, I won't be upset if we close this and go with another change.)

By the way, I believe this is fixed in the previous release, 2.17.0, but I don't understand where the bug is in the diff between that and 2.17.1.

[dearchap]

@julian7 Yes all the current flags do implement CategoryFlag but its possible that any user created custom flags may not in which case it will hit the second OPTIONS. But yes I agree most common category is "" so if we set that to default if custom flag doesnt implement CategoryFlag then we can get rid of the second OPTIONS.

[dearchap]

@aimichal Can you check the latest release and see if its fixed the problem ?

[aimichal]

@aimichal Can you check the latest release and see if its fixed the problem ?

Unfortunately, not. I encountered this when using 2.17.1. My repro, on MacOS:

package main

import (
	"log"

	"github.com/urfave/cli/v2"
)

func main() {
	app := &cli.App{
		Commands: []*cli.Command{
			{
				Name: "xxx",
				Flags: []cli.Flag{
					&cli.StringFlag{Name: "foo", Value: "foo", Usage: "foo"},
					&cli.StringFlag{Name: "bar", Value: "bar", Usage: "bar"},
				},
			},
		},
	}

	if err := app.Run([]string{"example", "xxx", "--help"}); err != nil {
		log.Fatal(err)
	}
}

Output has:

OPTIONS:
   --bar value  bar (default: "bar")--foo value  foo (default: "foo")

This is confirmed in the playground as well: https://go.dev/play/p/dThmnjhd1U3

[aimichal]

On the other hand, using the latest version in main branch (38 commits ahead of latest release, v2.17.1) works fine:

~/code/goplayground  $ go get github.com/urfave/cli/v2@main                                    
go: downloading github.com/urfave/cli/v2 v2.17.2-0.20221006152029-f64acc440459
go: upgraded github.com/urfave/cli/v2 v2.17.1 => v2.17.2-0.20221006152029-f64acc440459
~/code/goplayground  $ go build main.go && ./main                                              
NAME:
   main xxx

USAGE:
   main xxx [command options] [arguments...]

OPTIONS:
   --bar value  bar (default: "bar")
   --foo value  foo (default: "foo")
   --help, -h   show help (default: false)

So I'm closing this PR.