delve icon indicating copy to clipboard operation
delve copied to clipboard

Global flags are not all global

Open polinasok opened this issue 4 years ago • 5 comments
trafficstars

I am noticing this in the context of dlv-dap, but this is true for other modes as well (e.g. build flags with exec, attach, etc). Also, as new flags get added, it is not clear if they are global and always apply as well.

It might be nice to have a map of subcommands and applicable flags and use that to set them. But if it is too tedious to add almost-global flags multiple times for the individual commands, then perhaps each such flag should mention in the description what subcommands it doesn't apply to? Or maybe each subcommand that ignores it should print a warning that the flag is ignored (either because the feature is not yet supported or because it doesn't make sense at all)?

In particular, based on dlv dap --help:

Global Flags:

  • --accept-multiclient
    • not yet supported
    • if set by user, prints "Warning: accept-multiclient mode not supported with dap"
  • --allow-non-terminal-interactive
    • new flag that was not there when I added dapCmd
    • not applicable? print warning if set?
  • --api-version int
    • doesn't apply with dap
    • does a warning make sense given that there is a default, so it will always be printed?
    • add a note to the flag description that version is meaningless under dap?
  • --backend
    • passed to debugger.Config
    • default is not empty, so support via flags instead of launch/attach args
    • does it make more sense to support this via launch/attach args instead or also?
    • in that case, update the flag description/warning?
  • --build-flags
    • not supported
    • If set, prints "Warning: build flags ignored with dap; specify via launch/attach request instead"
  • --check-go-version
    • passed to debugger.Config
    • default is not unset, so support via flags instead of launch/attach args
  • --disable-aslr Disables address space randomization
    • new flag added after dapCmd
    • pass to debugger.Config?
  • --headless
    • dap is always headless, so this flag doesn't make sense
    • if set, "Warning: dap mode is always headless"
    • or should we always require it to be set explicitly because one day we might want to add a terminal frontend?
  • --init string Init file, executed by the terminal client.
    • "Warning: init file ignored with dap"
  • --listen string Debugging server listen address. (default "127.0.0.1:0")
    • in use
  • --log
    • in use
  • --log-dest string
    • in use
  • --log-output string
    • in use
  • --only-same-user
    • new flag added after dapCmd
    • how to handle?
  • --redirect
    • new flag added to dapCmd
    • pass to debugger.Config?
  • --wd
    • supported via launch args
    • if set, prints "Warning: working directory ignored with dap"

polinasok avatar Feb 24 '21 23:02 polinasok

/cc @suzmue

polinasok avatar Feb 24 '21 23:02 polinasok

For the record we can't move those flags to subcommands because it would be backwards incompatible. For example dlv --headless debug ... would no longer be valid.

aarzilli avatar Feb 25 '21 13:02 aarzilli

Sure makes sense. It would be nice to be more consistent for new flags, but the old ones are what they are, minus the clarity of when they are applicable, which can still be addressed with warnings, better flag descriptions, additional notes under --help.

polinasok avatar Feb 26 '21 07:02 polinasok

Should this stay open?

aarzilli avatar Dec 08 '21 14:12 aarzilli

Perhaps we keep this open until we've improved warnings on when flags won't actually be used? Might be worth to create a checklist in this issue to keep track (cc @polinasok).

derekparker avatar Feb 03 '22 21:02 derekparker