From c73f38cd547cccc7930cf2e302c835e3f424ffd4 Mon Sep 17 00:00:00 2001
From: Hugo Hromic <hhromic@gmail.com>
Date: Fri, 14 Jul 2023 20:12:47 +0100
Subject: [PATCH] Improve handling of version flag

* Only use/show builtin `--version` flag if args are versioned with a non-empty `Version()`
* If args define a `--version` flag, honor it and disable/hide the builtin version flag
* Only return `ErrVersion` when using the builtin version flag
---
 parse.go      | 17 ++++++++++++++---
 parse_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
 usage.go      |  9 ++++++++-
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/parse.go b/parse.go
index 63cfab3..0bdddc7 100644
--- a/parse.go
+++ b/parse.go
@@ -69,10 +69,10 @@ type command struct {
 	parent      *command
 }
 
-// ErrHelp indicates that -h or --help were provided
+// ErrHelp indicates that the builtin -h or --help were provided
 var ErrHelp = errors.New("help requested by user")
 
-// ErrVersion indicates that --version was provided
+// ErrVersion indicates that the builtin --version was provided
 var ErrVersion = errors.New("version requested by user")
 
 // for monkey patching in example code
@@ -591,6 +591,15 @@ func (p *Parser) process(args []string) error {
 		}
 	}
 
+	// determine if the current command has a version option spec
+	var hasVersionOption bool
+	for _, spec := range curCmd.specs {
+		if spec.long == "version" {
+			hasVersionOption = true
+			break
+		}
+	}
+
 	// process each string from the command line
 	var allpositional bool
 	var positionals []string
@@ -648,7 +657,9 @@ func (p *Parser) process(args []string) error {
 		case "-h", "--help":
 			return ErrHelp
 		case "--version":
-			return ErrVersion
+			if !hasVersionOption && p.version != "" {
+				return ErrVersion
+			}
 		}
 
 		// check for an equals sign, as in "--foo=bar"
diff --git a/parse_test.go b/parse_test.go
index 77a034f..06e7a76 100644
--- a/parse_test.go
+++ b/parse_test.go
@@ -1380,11 +1380,55 @@ func TestReuseParser(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestVersion(t *testing.T) {
+func TestNoVersion(t *testing.T) {
 	var args struct{}
-	err := parse("--version", &args)
-	assert.Equal(t, ErrVersion, err)
 
+	p, err := NewParser(Config{}, &args)
+	require.NoError(t, err)
+
+	err = p.Parse([]string{"--version"})
+	assert.Error(t, err)
+	assert.NotEqual(t, ErrVersion, err)
+}
+
+func TestBuiltinVersion(t *testing.T) {
+	var args struct{}
+
+	p, err := NewParser(Config{}, &args)
+	require.NoError(t, err)
+
+	p.version = "example 3.2.1"
+
+	err = p.Parse([]string{"--version"})
+	assert.Equal(t, ErrVersion, err)
+}
+
+func TestArgsVersion(t *testing.T) {
+	var args struct {
+		Version bool `arg:"--version"`
+	}
+
+	p, err := NewParser(Config{}, &args)
+	require.NoError(t, err)
+
+	err = p.Parse([]string{"--version"})
+	require.NoError(t, err)
+	require.Equal(t, args.Version, true)
+}
+
+func TestArgsAndBuiltinVersion(t *testing.T) {
+	var args struct {
+		Version bool `arg:"--version"`
+	}
+
+	p, err := NewParser(Config{}, &args)
+	require.NoError(t, err)
+
+	p.version = "example 3.2.1"
+
+	err = p.Parse([]string{"--version"})
+	require.NoError(t, err)
+	require.Equal(t, args.Version, true)
 }
 
 func TestMultipleTerminates(t *testing.T) {
diff --git a/usage.go b/usage.go
index 0498910..a9f9844 100644
--- a/usage.go
+++ b/usage.go
@@ -209,6 +209,7 @@ func (p *Parser) WriteHelpForSubcommand(w io.Writer, subcommand ...string) error
 // writeHelp writes the usage string for the given subcommand
 func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) {
 	var positionals, longOptions, shortOptions, envOnlyOptions []*spec
+	var hasVersionOption bool
 	for _, spec := range cmd.specs {
 		switch {
 		case spec.positional:
@@ -243,6 +244,9 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) {
 		}
 		for _, spec := range longOptions {
 			p.printOption(w, spec)
+			if spec.long == "version" {
+				hasVersionOption = true
+			}
 		}
 	}
 
@@ -259,6 +263,9 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) {
 		fmt.Fprint(w, "\nGlobal options:\n")
 		for _, spec := range globals {
 			p.printOption(w, spec)
+			if spec.long == "version" {
+				hasVersionOption = true
+			}
 		}
 	}
 
@@ -269,7 +276,7 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) {
 		short:       "h",
 		help:        "display this help and exit",
 	})
-	if p.version != "" {
+	if !hasVersionOption && p.version != "" {
 		p.printOption(w, &spec{
 			cardinality: zero,
 			long:        "version",