writ icon indicating copy to clipboard operation
writ copied to clipboard

Overridable flag

Open yegle opened this issue 8 years ago • 6 comments

E.g. if -a and -b are both specified then the last one specified will be used.

A workaround might be adding --mode option and specify --mode=a or --mode=b.

I'd propose to extend/change the OptionDecoder interface, where both option name and value are passed to the Decode function.

yegle avatar Feb 07 '16 10:02 yegle

Thanks for the suggestion! I'm going to mull it over a bit, but so far I like your idea!

bobziuchkovski avatar Feb 07 '16 15:02 bobziuchkovski

While I'm thinking about it, below is an example of how you could implement this using the current OptionDecoder interface. I wanted to share it in case you're blocked trying to implement your modes. However, I think I'm very likely to change the OptionDecoder interface as you suggested. I think it would be nice to be able to tackle the same use case without having to resort to explicit Command/Option construction and having to use two instances of OptionDecoder tied to the same underlying variable.

That said, here's how you can make it work if you're blocked:

package main

import (
    "fmt"
    "github.com/bobziuchkovski/writ"
    "os"
)

const (
    defaultMode mode = iota
    modeA
    modeB
)

type mode uint

func (m mode) String() string {
    switch m {
    case defaultMode:
        return "Default Mode"
    case modeA:
        return "Mode A"
    case modeB:
        return "Mode B"
    default:
        return "Invalid Mode"
    }
}

type modeDecoder struct {
    targetMode mode
    value      *mode
}

func (d modeDecoder) Decode(arg string) error {
    *d.value = d.targetMode
    return nil
}

type config struct {
    mode mode
}

func main() {
    cfg := &config{}
    cmd := writ.New("test", cfg)
    cmd.Help.Usage = "Usage: test [OPTION]... [ARG]..."
    cmd.Options = []*writ.Option{
        {
            Names:       []string{"a"},
            Description: "Set mode to a",
            Decoder:     modeDecoder{targetMode: modeA, value: &cfg.mode},
            Flag:        true,
        },
        {
            Names:       []string{"b"},
            Description: "Set mode to b",
            Decoder:     modeDecoder{targetMode: modeB, value: &cfg.mode},
            Flag:        true,
        },
    }
    general := cmd.GroupOptions("a", "b")
    general.Header = "General Options:"
    cmd.Help.OptionGroups = append(cmd.Help.OptionGroups, general)

    _, positional, err := cmd.Decode(os.Args[1:])
    if err != nil {
        cmd.ExitHelp(err)
    }

    fmt.Println("Positional args:", positional)
    fmt.Println("Mode:", cfg.mode)
}

bobziuchkovski avatar Feb 07 '16 16:02 bobziuchkovski

Hmm, never thought about reusing a decoder for multiple flags. I was thinking about a single flag with []string{"a", "b"} as Names field. I think your code works, I'll use this for now. Thank you for the hint.

Leave this issue open since you might want to add overridable flag eventually.

yegle avatar Feb 08 '16 04:02 yegle

Yeah, I thought about this a lot today, and I'm having second thoughts on the OptionDecoder interface change. I keep coming to the conclusion that the outlined use case, mutually exclusive modes where the last mode specified wins, is actually a weird case of permitting and potentially encouraging option ambiguity. It's kind of weird for me to think that -ba could mean something different than -ab. I realize some CLI apps behave this way, but I think it's better to fail-fast, telling the user they've specified conflicting options, than to silently drop/override one of the specified options.

In fact, in the past month, I know I've come across two different forum posts, one for qemu and one for ffmpeg, where the users in question were pulling their hair out trying to figure out what they were doing wrong, and in both case it came down to duplicating/overriding options and not realizing it. In the qemu case, someone specified -smp multiple times within a long command, forgetting they had done so. I can't remember what the case was for ffmpeg, but I know I saw one.

In any event, the example I shared is definitely functional and will do what you're describing, but I'm not sure I'm convinced it's a good thing to make "easier" to achieve. However, I am going to try to reduce some of the boilerplate for the explicit Command/Option construction in general.

bobziuchkovski avatar Feb 08 '16 05:02 bobziuchkovski

This is pretty common in open source code. E.g. ls on OS X:

The -1, -C, -x, and -l options all override each other; the last one specified determines the format used.

yegle avatar Feb 08 '16 17:02 yegle

Common, but not necessarily a good thing :smile:. I'll leave the issue open and think on it some more, but for now I don't know if the trade-off is worthwhile for the interface change. It seems like it might encourage funky design.

bobziuchkovski avatar Feb 08 '16 19:02 bobziuchkovski