Use a core flag parser other than stdlib flag to unlock wider range of arg types

[meatballhat]

tl;dr - By switching away from stdlib flag, we will be able to support a wider range of capabilities with significantly less workaround code.

Much of the code around here is arguably working around stdlib flag, for example:

• concept of commands
  • nesting of commands
  • commands with aliases
• persistent flags (in progress)
• flags with aliases
• flags of types not already supported in stdlib (via FlagSet.Var)
  • slice-specific (de)serialization hacks
  • other inconsistently-implemented (de)serialization hacks
• probably other stuff!

By building on top of stdlib flag, we are limited to two categories of string arguments:

• flags
  • must start with -
  • may accept zero or one argument
• non-flag string literals
  • must not start with -
  • no additional help from stdlib flag 🤷🏼

There are many other command line argument parsers available in the Go third-party package ecosystem such as:

• pflag, a lower-level library that provides a compatibility layer with stdlib flag
  • typically seen via this fork which isn't being actively maintained
• cobra, a holistic command and flag building solution that encourages imperative construction of command contexts (depends on pflag)
• kong, a holistic command and flag building solution with strong support for deriving applications from nested struct variables
• argparse, an imperative solution which aims to provide functionality like the Python stdlib argparse

[jtagcat]

(Moving from #833 (comment) to here)

[jtagcat]

flags: must start with -

This is reasonable imo, I haven't seen any use for not starting (non-argument) flags without -. It also majorly simplifies parsing.

[meatballhat]

flags: must start with -

This is reasonable imo, I haven't seen any use for not starting (non-argument) flags without -. It also majorly simplifies parsing.

@jtagcat it's a unix-y-centric way of working, imho. For example, in Windows there's strong precedent for / as prefix and : as assignment like /{flag}:{value}, although this also conflicts with the idea of "compound short flags" 🤷🏼 I'd like to provide a flexible way to allow for different argument styles if possible, e.g. how argparse does it.

[jtagcat]

Ok, fair point.

I'd say multiple prefixes at once gets unreasonably complex. Yet everything is possible by swapping out (not adding to) the prefix and/or the non-space delimiter (=) and/or boolean for enabling compound short flags. Multiple parse calls with different delimiters gets you any result you could want.

For arguments without a key (eg. @file.txt), the caller (or a library wrapping harg) can just do a for range against (parsed) args with strings.HasPrefix() tailored to the use-case.

Performance of multiple parses isn't an issue. First, arguments are only parsed once (on app startup). With modern processors, multiple calls may even be more performant vs a messy monolithic do-all parser.

But I'd not touch the topic right now at all. A bad standard is better than 5 standards. In my opinion, the spec I have in FORMAT.md is fairly good, well-known, and used. Even (new) Microsoft CLI utilities use GNU-like arguments.

Everone is free to fork (and wrap the library or whole cli binary) for their own niche. I want to make the codebase maintainable (and workable) for as many as possible, including myself. One niche use-case just isn't worth it.

I want to keep it as simple as possible. There isn't anything we are removing compared to stdlib flags. Right now, getting v3 released is the priority.

[meatballhat]

@jtagcat I appreciate the points you're making.

I'd love to see what an integration of harg for the purpose of addressing this issue looks like, if you're up for doing that work.

I'm personally interested in exploring this approach, if only to better understand how the urfave/cli layer can change.

[dearchap]

@meatballhat @jtagcat Both of you make good points. I think as software devs we'd like to make software perfect - covers all use cases, very performant, infintely extensible and so on. While a complete new parser is good, we need to see how much we can stretch stdlib flag to the max to cover most of the use cases. We cannot satisfy every user's use cases so its better to list what features beyond what urfave/cli/v2 offers do we want to achieve.

[meatballhat]

@dearchap Are you suggesting that this issue is not worth pursuing (yet)? 😅 imho the workarounds in place to support nested commands and compound short flags are indications that we've already gone past the point of stretching stdlib flag to the max. Or do I misunderstand what you mean? ❤️

[dearchap]

@meatballhat What are the most requested features of this library ?

1. Global/Persistent flags
2. Help consistency(and with being able to do help without required flags)

if we have those two knocked out, rest of the features are icing on the cake no ?

[jtagcat]

Update: https://github.com/jtagcat/harg is tested and ready to integrate.

henv (environment variables) seems surprisingly straight-forward to implement. Would likely have to restructure the project files, though.

I think I'd like to keep help text, defaults, and bash completion outside the package. I'll dig around this cli package before, to see where it makes the most sense to implement at.

@dearchap @meatballhat requesting review if you have the time.

[meatballhat]

@jtagcat imho https://github.com/jtagcat/harg is lovely as heck and I would very much like to see what an integration with urfave/cli/v3 looks like 😁 Is this work you're planning to do soon?

[jtagcat]

@meatballhat: Soon, but not soon. I have half a workday today, then maybe next Sunday, then longer periods starting around 18. dec.

Honestly, it plays such a major role in the cli package, a complete rewrite (creatively egoistically named hcli) for v3 might be doable.

What components are there?

• Flags wrapper:
  • Help prompt
    • Version: encourage setting compile-time variables, such as git hash. https://blog.alexellis.io/inject-build-time-vars-golang/ If undocumented, could lead to byte-level unreproducible builds, but it's way better than incrementing versions manually. ¹, ²
  • Shell completions ²
  • Default values
    • In Flag, replace Required bool with Action, renamed to Condition. It is trivial to write common implementations with generic functions. The library can provide commonly used ones, such as NotDefault (must be set by user, --foo="" is valid) and NotEmpty. In most cases, omitted or looking like cli.StringFlag{Name: "foo", Condition: cli.NotEmpty}.
      Ideas for changes I've had so far. I've asked around for feedback, and clarifying what Required did come up.
  • Validation (after type conversion)
  • Supporting yaml with nested flag definition structure and/or exposing setting flags in context ²
• Context
  • Signals ²
    • Make it clear on how interrupts are expected to be handled
  • Exit codes ²
    • Documenting and handling exit codes: Exit code caller should reference an already-defined code, similar to calling up a flag, not just 'exit 1'. ²
• Commands
  • Before/After
  • Subcommands
    • Flag-dependant subcommands: Quick example: $ app → ls $(pwd) $ app <flag> → cd <flag> ²

It should also include everything in the v3 milestone. Anything I missed? The only parts I expect to get stuck on are shell completions, and yaml. They are not critical for a v3 beta, though.

Footnotes

1. $ kubectl version
   Client Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.4", GitCommit:"872a965c6c6526caa949f0c6ac028ef7aff3fb78", GitTreeState:"clean", BuildDate:"2022-11-09T13:36:36Z", GoVersion:"go1.19.3", Compiler:"gc", Platform:"linux/amd64"}
   

   Implementation:

   var BuildInfoGitCommit string
   

   export GIT_COMMIT=$(git rev-list -1 HEAD) && \
   go build -ldflags "-X cli.BuildInfoGitCommit=$GIT_COMMIT"
   

   ↩

2. Not needed for a functional beta release. No changes are necessary for adding it without breaking changes. ↩ ↩² ↩³ ↩⁴ ↩⁵ ↩⁶ ↩⁷

[meatballhat]

@jtagcat do I understand correctly that you are recommending against integrating jtagcat/harg directly with this library, but instead to make a jtagcat/hcli project that uses it, and then use jtagcat/hcli in this library?

[dearchap]

@jtagcat could you provide some examples of how users would use the harg library ? Maybe a definition for a int flag and a float slice . Thanks

[jtagcat]

@jtagcat do I understand correctly that you are recommending against integrating jtagcat/harg directly with this library, but instead to make a jtagcat/hcli project that uses it, and then use jtagcat/hcli in this library?

I meant urfave/cli@v3 = hcli, maybe. I'm not recommending, just had some thoughts.

[jtagcat]

@jtagcat could you provide some examples of how users would use the harg library? Maybe a definition for a int flag and a float slice . Thanks

Users using harg through cli:

One way would be to do as it is now: Define flags (and aliases, etc), and then retrieve by string. Just the underlining library differs. (I would expect a flag to be available for all subcommands unless explicitly marked as local.)

I don't know if it is ergonomically possible to type the lookup string, as it is in harg tests:

key1 := "foo"
defs := harg.Definitions{key1: {Type: harg.String}}

defs.Parse()

s, _ := defs[key1].String()

I probably need some time to think about it.

---

If you meant direct use of harg, see https://github.com/jtagcat/harg/blob/main/parse_test.go#L16 (or example at https://pkg.go.dev/github.com/jtagcat/harg#Definitions.Parse)

[jtagcat]

What I came up with atm:

One thing maybe needing explaining is Source. It defines the priority (opt overrides env, or other way around).

Value-typing is still available:

var (
  flagFoo = "foo"
)

_ = hcli.Command{
	Flags: []hcli.Flag{
		hcli.StringFlag{Options: []string{flagFoo, "f"}},
	}
	Action: ... {
		ctx.SlString(flagFoo)
	}
	...
}

This provides completion and validation in the IDE, and ctrl-clicking (jump to definition, or show all uses).

[jtagcat]

I have a chicken and egg problem. It originates from boolean flags not consuming the next argument, thus options in general maybe consuming the next argument, what is a problem when determining whether a subcommand is a value for an option, or a subcommand.

One could expect all of the following to work (using only long options for sake of simplicity):

$ hello subcommand --world val # subcommand with world=val
$ hello subcommand --yes # boolean yes
$ hello --world val subcommand --world val # depending on globals, subcommand:[val val] or global:val, world:val
$ hello --yes subcommand # boolean yes
$ hello --world subcommand # root command with world=subcommand

I would prefer globals to be explicitly defined. Helptext can be managed with categories (and optionally hiding globals in subcommand helptext). This would mean that globals have to be defined in all subcommands (flag definitions can be reused with variables).
Explicit globals would mean that order does not matter ($ hello --world val subcommand --world val, where --world is the same). The problem here is that the parser does not know what subcommand flags it parses against before parsing (--world could be a bollean, like --yes). This could be solved by restricting flags to be allowed only after subcommands ($ hello --world val subcommand would be invalid).

Implicit globals would mean that flags have a recursive flag in their definition, and only ever defined once. While parsing, the arguments are parsed against root command's definitions. When a subcommand (choke) is found, everything until the subcommand is parsed again for only recursive definitions. Next, the subcommand arguments are parsed (recursively). Option order would always matter.

Right now, implicit globals sound better. This might need more examples.

Cobra uses restrictive explicit style, Docker ordered implicit. Kubernetes stuff uses double-defined unordered ordered implicit globals. Meaning that global options are defined on the global level and local level. Local flags before subcommands are disallowed.

Double-defined implicit globals could in theory be implemented (generically, consumable simply as a cli library), with the only downside being half-implicitness (flags are defined globally, but their action has to be always remembered and defined in subcommands). This could be solved with linting.

tldr

I think recursive double-defined implicit ordered globals is actually doable, roadblock solved by writing a letter to a rubber ducky. I need more time.

[jtagcat]

Progress at https://github.com/jtagcat/harg/tree/hcli. It's coming together beautifully imo, needs a workweek more (x2 for unused feature creep).

If there is interest, I still would like to make it a future version of urfave/cli.

[dearchap]

@jtagcat We have support for persistent flags in v3 right now. So if you mark a flag as persistent it will be available to all subsequent sub commands. if sub command defines same flag it will take precedence over persistent one.

[dearchap]

@jtagcat Right now we have 2 competing arg parsers, yours and @meatballhat 's argh. At some point we have to make a decision which path is preferred.

[skelouse]

Two requested parses:

1. Positional arguments should be parsed no matter the position, referencing the commands and flags to determine what is positional and what is a flag argument
2. Back-propagated persistence without adding to help menu on higher commands

In my current project I've built an inefficient args parser which moves flags and positional arguments to their correct positions before sending to app.Run(...).

Example: Given an App

&cli.App{
    Commands: []*cli.Command{
        {
            Name: "function",
            ArgsUsage: "<name>",
            Flags: []cli.Flag{
                &cli.StringFlag{
                    Name:    "command",
                },
            },
        },
    },
}

Arguments : prog function functionName --command cmd
Parsed    : prog function --command cmd functionName

Arguments : prog --command cmd function
Parsed    : prog function --command cmd

Whenever we release current iteration it will be here: https://github.com/taubyte/tau/tree/main/cli/args

[skelouse]

Above is now released, you can see my tests here: https://github.com/taubyte/tau/blob/main/cli/args/args_test.go

[meatballhat]

@skelouse sorry for the long delay! I think your requests are accounted for, but I'll definitely want more feedback once this is closer. I've been occupied with other life adventures since ~January-ish but I've got some vacation time coming up 🤞🏼

[meatballhat]

I have lost focus on this more than once, and IIUC others in threads here have also moved on, so I'd like to propose that this is not a blocker for v3 release. @urfave/cli WDYT?

[dearchap]

I agree. We probably handle most of the use cases for a cli utility. Refitting a new parser is a nice engineering project but adds only incremental value