jawher / mow.cli

A versatile library for building CLI applications in Go
MIT License
872 stars 55 forks source link

Provide possibility of passing pointer to value for options or argument #83

Closed silasdavis closed 5 years ago

silasdavis commented 5 years ago

I was recently porting an command from Cobra. It looked like:

var cfg = config.DefaultFlags()

func init() {
    ventCmd.Flags().StringVar(&cfg.DBAdapter, "db-adapter", cfg.DBAdapter, "Database adapter, 'postgres' or 'sqlite' are fully supported")
    ventCmd.Flags().StringVar(&cfg.DBURL, "db-url", cfg.DBURL, "PostgreSQL database URL or SQLite db file path")
    ventCmd.Flags().StringVar(&cfg.DBSchema, "db-schema", cfg.DBSchema, "PostgreSQL database schema (empty for SQLite)")
    ventCmd.Flags().StringVar(&cfg.GRPCAddr, "grpc-addr", cfg.GRPCAddr, "Address to connect to the Hyperledger Burrow gRPC server")
    ventCmd.Flags().StringVar(&cfg.HTTPAddr, "http-addr", cfg.HTTPAddr, "Address to bind the HTTP server")
    ventCmd.Flags().StringVar(&cfg.LogLevel, "log-level", cfg.LogLevel, "Logging level (error, warn, info, debug)")
    ventCmd.Flags().StringVar(&cfg.SpecFile, "spec-file", cfg.SpecFile, "SQLSol json specification file full path")
    ventCmd.Flags().StringVar(&cfg.AbiFile, "abi-file", cfg.AbiFile, "Event Abi specification file full path")
    ventCmd.Flags().StringVar(&cfg.AbiDir, "abi-dir", cfg.AbiDir, "Path of a folder to look for event Abi specification files")
    ventCmd.Flags().StringVar(&cfg.SpecDir, "spec-dir", cfg.SpecDir, "Path of a folder to look for SQLSol json specification files")
    ventCmd.Flags().BoolVar(&cfg.DBBlockTx, "db-block", cfg.DBBlockTx, "Create block & transaction tables and persist related data (true/false)")
    ventCmd.Flags().BoolVarP(&printVersion, "version", "v", false, "Print the full Vent version (note this version matches the version of Burrow from the same source tree)")
}

In mow.cli since I have no way of providing it with a pointer to fill I had to do this:

        cfg := config.DefaultFlags()

        dbAdapterOpt := cmd.StringOpt("db-adapter", cfg.DBAdapter, "Database adapter, 'postgres' or 'sqlite' are fully supported")
        dbURLOpt := cmd.StringOpt("db-url", cfg.DBURL, "PostgreSQL database URL or SQLite db file path")
        dbSchemaOpt := cmd.StringOpt("db-schema", cfg.DBSchema, "PostgreSQL database schema (empty for SQLite)")
        grpcAddrOpt := cmd.StringOpt("grpc-addr", cfg.GRPCAddr, "Address to connect to the Hyperledger Burrow gRPC server")
        httpAddrOpt := cmd.StringOpt("http-addr", cfg.HTTPAddr, "Address to bind the HTTP server")
        logLevelOpt := cmd.StringOpt("log-level", cfg.LogLevel, "Logging level (error, warn, info, debug)")
        specFileOpt := cmd.StringOpt("spec-file", cfg.SpecFile, "SQLSol json specification file full path")
        abiFileOpt := cmd.StringOpt("abi-file", cfg.AbiFile, "Event Abi specification file full path")
        abiDirOpt := cmd.StringOpt("abi-dir", cfg.AbiDir, "Path of a folder to look for event Abi specification files")
        specDirOpt := cmd.StringOpt("spec-dir", cfg.SpecDir, "Path of a folder to look for SQLSol json specification files")
        dbBlockTxOpt := cmd.BoolOpt("db-block", cfg.DBBlockTx, "Create block & transaction tables and persist related data (true/false)")

        cmd.Before = func() {
            // Rather annoying boilerplate here... but there is no way to pass mow.cli a pointer for it to fill your values for you
            cfg.DBAdapter = *dbAdapterOpt
            cfg.DBURL = *dbURLOpt
            cfg.DBSchema = *dbSchemaOpt
            cfg.GRPCAddr = *grpcAddrOpt
            cfg.HTTPAddr = *httpAddrOpt
            cfg.LogLevel = *logLevelOpt
            cfg.SpecFile = *specFileOpt
            cfg.AbiFile = *abiFileOpt
            cfg.AbiDir = *abiDirOpt
            cfg.SpecDir = *specDirOpt
            cfg.DBBlockTx = *dbBlockTxOpt
        }

Not the end of the world but rather annoying boilerplate. I wonder if mow.cli could support something similar to Cobra and provide the option to pass in a pointer. I note that the base functions exist in the internal package.

Perhaps refactoring (in commands.go) to the following wouldn't be too bad:

func (c *Cmd) String(p StringParam) *string {
    into := new(string)
    return c.StringPtr(p, into)
}

func (c *Cmd) StringPtr(p StringParam, into *string) *string {
    value := values.NewString(into, p.value())

    switch x := p.(type) {
    case StringOpt:
        c.mkOpt(container.Container{Name: x.Name, Desc: x.Desc, EnvVar: x.EnvVar, HideValue: x.HideValue, Value: value, ValueSetByUser: x.SetByUser})
    case StringArg:
        c.mkArg(container.Container{Name: x.Name, Desc: x.Desc, EnvVar: x.EnvVar, HideValue: x.HideValue, Value: value, ValueSetByUser: x.SetByUser})
    default:
        panic(fmt.Sprintf("Unhandled param %v", p))
    }

    return into
}
jawher commented 5 years ago

Yep, shouldn't be too hard to add.

I'll try to start working on this & hopefully it should be ready soon*(ish) !