Repository: danielgtaylor/restish
Pull request #96 (2022-04-11 17:33:05)

fix: simplify option handling, send explicitly passed defaults
This addresses an issue brought up in #48, where it is difficult to send the default value for an argument (query/header param). Instead of comparing to the defined default or zero value, the code now considers whether an argument was passed on the command line or not and should allow sending of default values when explicitly passed to the command.

codecov[bot]: # [Codecov](https://codecov.io/gh/danielgtaylor/restish/pull/96?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor) Report
> Merging [#96](https://codecov.io/gh/danielgtaylor/restish/pull/96?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor) (96fdbbc) into [main](https://codecov.io/gh/danielgtaylor/restish/commit/c094c86acb1e84fdc803339b6fd68c68aa22dc9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor) (c094c86) will **increase** coverage by `0.35%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
+ Coverage   63.45%   63.81%   +0.35%     
==========================================
  Files          21       21              
  Lines        2200     2194       -6     
==========================================
+ Hits         1396     1400       +4     
+ Misses        649      639      -10     
  Partials      155      155              
```


| [Impacted Files](https://codecov.io/gh/danielgtaylor/restish/pull/96?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor) | Coverage Δ | |
|---|---|---|
| [cli/operation.go](https://codecov.io/gh/danielgtaylor/restish/pull/96/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor#diff-Y2xpL29wZXJhdGlvbi5nbw==) | `77.41% <100.00%> (+12.20%)` | :arrow_up: |
| [cli/param.go](https://codecov.io/gh/danielgtaylor/restish/pull/96/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor#diff-Y2xpL3BhcmFtLmdv) | `88.31% <100.00%> (+0.15%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/danielgtaylor/restish/pull/96?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/danielgtaylor/restish/pull/96?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor). Last update [c094c86...96fdbbc](https://codecov.io/gh/danielgtaylor/restish/pull/96?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+G.+Taylor).


Patch:
diff --git a/cli/operation.go b/cli/operation.go
index de849ed..2928a27 100644
--- a/cli/operation.go
+++ b/cli/operation.go
@@ -6,7 +6,6 @@ import (
 	"log"
 	"net/http"
 	"net/url"
-	"reflect"
 	"strings"
 
 	"github.com/gosimple/slug"
@@ -74,18 +73,13 @@ func (o Operation) command() *cobra.Command {
 
 			query := url.Values{}
 			for _, param := range o.QueryParams {
-				flag := flags[param.Name]
-				if reflect.ValueOf(flag).Elem().Interface() == param.Default {
-					// No need to send the default value. Just skip it.
-					continue
-				}
-
-				if param.Default == nil && reflect.ValueOf(flag).Elem().IsZero() {
-					// No explicit default, so the implied default is the zero value.
-					// Again no need to send that default, so we skip.
+				if !cmd.Flags().Changed(param.OptionName()) {
+					// This option was not passed from the shell, so there is no need to
+					// send it, even if it is the default or zero value.
 					continue
 				}
 
+				flag := flags[param.Name]
 				for _, v := range param.Serialize(flag) {
 					query.Add(param.Name, v)
 				}
@@ -118,26 +112,12 @@ func (o Operation) command() *cobra.Command {
 
 			headers := http.Header{}
 			for _, param := range o.HeaderParams {
-				rv := reflect.ValueOf(flags[param.Name]).Elem()
-				if rv.Interface() == param.Default {
-					// No need to send the default value. Just skip it.
+				if !cmd.Flags().Changed(param.OptionName()) {
+					// This option was not passed from the shell, so there is no need to
+					// send it, even if it is the default or zero value.
 					continue
 				}
 
-				if param.Default == nil {
-					if rv.IsZero() {
-						// No explicit default, so the implied default is the zero value.
-						// Again no need to send that default, so we skip.
-						continue
-					}
-
-					if rv.Kind() == reflect.Slice && rv.Len() == 0 {
-						// IsZero() above fails for empty arrays, so if it's empty let's
-						// ignore it.
-						continue
-					}
-				}
-
 				for _, v := range param.Serialize(flags[param.Name]) {
 					headers.Add(param.Name, v)
 				}
diff --git a/cli/param.go b/cli/param.go
index 4d1e63b..7dacd0f 100644
--- a/cli/param.go
+++ b/cli/param.go
@@ -95,14 +95,18 @@ func (p Param) Serialize(value interface{}) []string {
 	return nil
 }
 
-// AddFlag adds a new option flag to a command's flag set for this parameter.
-func (p Param) AddFlag(flags *pflag.FlagSet) interface{} {
+// OptionName returns the commandline option name for this parameter.
+func (p Param) OptionName() string {
 	name := p.Name
 	if p.DisplayName != "" {
 		name = p.DisplayName
 	}
-	name = strcase.ToDelimited(name, '-')
+	return strcase.ToDelimited(name, '-')
+}
 
+// AddFlag adds a new option flag to a command's flag set for this parameter.
+func (p Param) AddFlag(flags *pflag.FlagSet) interface{} {
+	name := p.OptionName()
 	def := p.Default
 
 	switch p.Type {


Test patch:
diff --git a/cli/operation_test.go b/cli/operation_test.go
index c3f522a..6eb9e05 100644
--- a/cli/operation_test.go
+++ b/cli/operation_test.go
@@ -13,9 +13,16 @@ import (
 func TestOperation(t *testing.T) {
 	defer gock.Off()
 
-	gock.New("http://example2.com").Get("/prefix/test/id1").MatchParam("search", "foo").Reply(200).JSON(map[string]interface{}{
-		"hello": "world",
-	})
+	gock.
+		New("http://example2.com").
+		Get("/prefix/test/id1").
+		MatchParam("search", "foo").
+		MatchParam("def3", "abc").
+		MatchHeader("Accept", "application/json").
+		Reply(200).
+		JSON(map[string]interface{}{
+			"hello": "world",
+		})
 
 	op := Operation{
 		Name:        "test",
@@ -51,6 +58,29 @@ func TestOperation(t *testing.T) {
 				Description: "desc",
 				Default:     "",
 			},
+			{
+				Type:        "string",
+				Name:        "def3",
+				DisplayName: "def3",
+				Description: "desc",
+				Default:     "abc",
+			},
+		},
+		HeaderParams: []*Param{
+			{
+				Type:        "string",
+				Name:        "Accept",
+				DisplayName: "Accept",
+				Description: "desc",
+				Default:     "application/json",
+			},
+			{
+				Type:        "string",
+				Name:        "Accept-Encoding",
+				DisplayName: "Accept-Encoding",
+				Description: "desc",
+				Default:     "gz",
+			},
 		},
 	}
 
@@ -63,9 +93,9 @@ func TestOperation(t *testing.T) {
 	capture := &strings.Builder{}
 	Stdout = capture
 	Stderr = capture
-	cmd.SetOut(Stdout)
+	cmd.SetOutput(Stdout)
 	viper.Set("rsh-server", "http://example2.com/prefix")
-	cmd.Flags().Parse([]string{"--search=foo"})
+	cmd.Flags().Parse([]string{"--search=foo", "--def-3=abc", "--accept=application/json"})
 	cmd.Run(cmd, []string{"id1"})
 
 	assert.Equal(t, "HTTP/1.1 200 OK\nContent-Type: application/json\n\n{\n  hello: \"world\"\n}\n", capture.String())