Fixes urfave/cli#1648

[palsivertsen]

Remove trim of whitespace in StringSliceFlag values, matching the behavior of StringFlag.

What type of PR is this?

• bug

What this PR does / why we need it:

See #1648

Which issue(s) this PR fixes:

Fixes #1648

Special notes for your reviewer:

I'm uncertain if the trim behavior was intentional or not as there was no tests covering it.

Testing

Added test that verifies that parsing the same value usingStringFlag and StringSliceFlag results in equal values.

Release Notes

`StringSliceFlag` no nonger trims space from values, matching the behavior of `StringFlag`.

[dearchap]

@palsivertsen Thanks for the excellent PR. Do you think we should add an option whether to trim space or not ?

[palsivertsen]

Do you think we should add an option whether to trim space or not ?

No. Users can call strconv.TrimSpace directly if needed.

[skelouse]

Do you think we should add an option whether to trim space or not ?

No. Users can call strconv.TrimSpace directly if needed.

This PR broke any code, including mine that was relying on strings to be trimmed by default.

[dearchap]

@skelouse @palsivertsen Can one of you submit a PR to include the option ? Thanks

[skelouse]

@skelouse @palsivertsen Can one of you submit a PR to include the option ? Thanks

IMO the option should be NoTrimSpace or KeepSpaces v3 could be TrimSpace

I'll leave it to @palsivertsen as he is relying on the default functionality to not trim.

[palsivertsen]

@skelouse @dearchap I'll play around with some designs. Should we open a separate issue for this, or reuse #1648 ?

[skelouse]

@palsivertsen Thanks for the quick response! Just reuse, it's still the same issue.

Edit: I saw another place that was trimming spaces, may help with your use case.

if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
	for _, s := range f.separator.flagSplitMultiValues(val) {
		if err := setValue.Set(strings.TrimSpace(s)); err != nil {
			return fmt.Errorf("could not parse %q as string value from %s for flag %s: %s", val, source, f.Name, err)
		}
	}

	// Set this to false so that we reset the slice if we then set values from
	// flags that have already been set by the environment.
	setValue.hasBeenSet = false
	f.HasBeenSet = true
}

[palsivertsen]

@skelouse Thanks!

Any idea where to put such a flag? My first instinct was to add it to the StringSliceFlag struct so that one could use it like so:

StringSliceFlag{Name: "slice", NoTrimSpace: true}

But the StringSliceFlag struct is generated.

[skelouse]

@palsivertsen I had the same issue with v2, I'm not exactly sure how they are generated. Maybe someone else can provide input @urfave/cli on how to edit the generated structs for v2.

[palsivertsen]

@skelouse I figured out how to add the field to generated structs. Here's what I've got so far:
https://github.com/palsivertsen/urfave-cli/commit/12b6187e7f5a91cfbaaa123ff2debdc7da4c76e9