StringSliceFlags has strange behavior

[KraftyKai]

1. When supplying StringSliceFlag with a default Value, the default value remains in the returned slice even when a value is manually applied. Example:
   Using:

cli.StringSliceFlag{  
        Name: "Hostnames, n", 
        Usage: "`HOSTNAME`s to listen to.", 
        Value: &cli.StringSlice{"localhost"},
}

Running invocation -n somehost returns [localhost somehost] but I would expect it to return only [somehost].

2. This can be fixed with a simple enough hack by using context.IsSet, but it turns out there is a bug here too. If I use the above invocation of invocation -n somehost then context.IsSet("Hostnames") == false, however, context.IsSet("n") == true. This is different from the behavior of the default values, such as Int or String where IsSet would return true whether the alternative switchname was used or not.

[michaeljs1990]

This is I imagine some unintended and untested behavior currently. I only tested it in the master branch with the following but put together a small command for easy validation later. I agree that both the short and the long form of the flag should show up as IsSet in the case you have shown above. I'm trying to think through an example where this would cause unintended behavior but am not able to at the moment.

I guess the easy work around for the time being would be to to check if both are set and then just use the values appended to each other. append(c.StringSlice("n"), c.StringSlice("Hostname")...). This still doesn't fix the issue with localhost being set unfortunately which is hopefully an easy fix but I will need to look into it a bit more.

package main

import (
        "fmt"
        "log"
        "os"

        "github.com/urfave/cli"
)

func main() {
        app := cli.NewApp()

        app.Flags = []cli.Flag{
                cli.StringSliceFlag{
                        Name:  "Hostnames, n",
                        Usage: "`HOSTNAME`s to listen to.",
                        Value: &cli.StringSlice{"localhost"},
                },
                cli.StringFlag{
                        Name:  "lang, l",
                        Value: "english",
                        Usage: "language for the greeting",
                },
        }

        app.Action = func(c *cli.Context) error {
                fmt.Println("Hostname is set ...", c.IsSet("Hostnames"))
                fmt.Println("n is set ...", c.IsSet("n"))

                fmt.Println("land is set ...", c.IsSet("lang"))
                fmt.Println("l is set ...", c.IsSet("l"))
                return nil
        }

        err := app.Run(os.Args)
        if err != nil {
                log.Fatal(err)
        }
}

myuser@gentoo ~/code/collins-go-cli $ go run lol.go -n somehost
Hostname is set ... false
n is set ... true
land is set ... false
l is set ... false

Here is conformation on the above that non slice values do work as you would expect.

myuser@gentoo ~/code/collins-go-cli $ go run lol.go -Hostnames somehost -l hi
Hostname is set ... true
n is set ... false
land is set ... true
l is set ... true

[iwilltry42]

That also got me super confused today, glad I'm not the only one 😅

[Yoone]

Same problem on my end. It also prevents me from using Required: true as it will sometimes fail for the same reason.

[stale]

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.

[coilysiren]

Can someone put together a feature request or bug report (templates are here) describing what should actually be done here? An issue titled "X has strange behavior" is hard to triage.

cc @KraftyKai @michaeljs1990 @iwilltry42 @Yoone

[stale]

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]

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]

Closing this as it has become stale.

[meatballhat]

As far as I can tell, this is no longer a problem on current v2. Modified from @michaeljs1990 example:

package main

import (
        "fmt"
        "log"
        "os"

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

func main() {
        app := &cli.App{
                Flags: []cli.Flag{
                        &cli.StringSliceFlag{
                                Name:    "Hostnames",
                                Aliases: []string{"n"},
                                Usage:   "`HOSTNAME`s to listen to.",
                                Value:   cli.NewStringSlice("localhost"),
                        },
                        &cli.StringFlag{
                                Name:    "lang",
                                Aliases: []string{"l"},
                                Value:   "english",
                                Usage:   "language for the greeting",
                        },
                },
                Action: func(c *cli.Context) error {
                        fmt.Println("Hostname is set ...", c.IsSet("Hostnames"))
                        fmt.Println("n is set ...", c.IsSet("n"))

                        fmt.Println("land is set ...", c.IsSet("lang"))
                        fmt.Println("l is set ...", c.IsSet("l"))
                        return nil
                },
        }

        if err := app.Run(os.Args); err != nil {
                log.Fatal(err)
        }
}

$ go run ~/tmp/urfave-cli-790b.go -Hostnames somehost -l hi
Hostname is set ... true
n is set ... true
land is set ... true
l is set ... true

Please holler if this problem is still affecting you on v2 🙇🏼