From 5ad7054788c0610a82016c901fab093195dc49d3 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Wed, 24 Nov 2021 17:09:03 -0500 Subject: [PATCH] gopls: add long forms for all the 1 rune flags Also modify the printing so that it groups flags that work on the same value into the same flag usage line. For #41860 Change-Id: I97fdd3e64f24b22c15780b636789f512eb46ed2c Reviewed-on: https://go-review.googlesource.com/c/tools/+/367838 Trust: Ian Cottrell Run-TryBot: Ian Cottrell gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley --- internal/lsp/cmd/cmd.go | 32 ++++++++++++-- internal/lsp/cmd/format.go | 6 +-- internal/lsp/cmd/imports.go | 4 +- internal/lsp/cmd/references.go | 2 +- internal/lsp/cmd/rename.go | 4 +- internal/lsp/cmd/suggested_fix.go | 6 +-- internal/lsp/cmd/usage/fix.hlp | 9 ++-- internal/lsp/cmd/usage/format.hlp | 9 ++-- internal/lsp/cmd/usage/imports.hlp | 6 ++- internal/lsp/cmd/usage/references.hlp | 3 +- internal/lsp/cmd/usage/rename.hlp | 6 ++- internal/lsp/cmd/usage/usage.hlp | 5 ++- internal/tool/tool.go | 61 +++++++++++++++++---------- 13 files changed, 102 insertions(+), 51 deletions(-) diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go index e23bca45e5..e67a966cda 100644 --- a/internal/lsp/cmd/cmd.go +++ b/internal/lsp/cmd/cmd.go @@ -16,6 +16,7 @@ import ( "log" "os" "reflect" + "sort" "strings" "sync" "text/tabwriter" @@ -63,10 +64,10 @@ type Application struct { Remote string `flag:"remote" help:"forward all commands to a remote lsp specified by this flag. With no special prefix, this is assumed to be a TCP address. If prefixed by 'unix;', the subsequent address is assumed to be a unix domain socket. If 'auto', or prefixed by 'auto;', the remote address is automatically resolved based on the executing environment."` // Verbose enables verbose logging. - Verbose bool `flag:"v" help:"verbose output"` + Verbose bool `flag:"v,verbose" help:"verbose output"` // VeryVerbose enables a higher level of verbosity in logging output. - VeryVerbose bool `flag:"vv" help:"very verbose output"` + VeryVerbose bool `flag:"vv,veryverbose" help:"very verbose output"` // Control ocagent export of telemetry OCAgent string `flag:"ocagent" help:"the address of the ocagent (e.g. http://localhost:55678), or off"` @@ -138,9 +139,32 @@ command: // this is a slightly modified version of flag.PrintDefaults to give us control func printFlagDefaults(s *flag.FlagSet) { + var flags [][]*flag.Flag + seen := map[flag.Value]int{} s.VisitAll(func(f *flag.Flag) { + if i, ok := seen[f.Value]; !ok { + seen[f.Value] = len(flags) + flags = append(flags, []*flag.Flag{f}) + } else { + flags[i] = append(flags[i], f) + } + }) + for _, entry := range flags { + sort.SliceStable(entry, func(i, j int) bool { + return len(entry[i].Name) < len(entry[j].Name) + }) var b strings.Builder - fmt.Fprintf(&b, " -%s", f.Name) // Two spaces before -; see next two comments. + for i, f := range entry { + switch i { + case 0: + b.WriteString(" -") + default: + b.WriteString(",-") + } + b.WriteString(f.Name) + } + + f := entry[0] name, usage := flag.UnquoteUsage(f) if len(name) > 0 { b.WriteString(" ") @@ -164,7 +188,7 @@ func printFlagDefaults(s *flag.FlagSet) { } } fmt.Fprint(s.Output(), b.String(), "\n") - }) + } } // isZeroValue is copied from the flags package diff --git a/internal/lsp/cmd/format.go b/internal/lsp/cmd/format.go index 458df5d1e9..2d0f3f7c37 100644 --- a/internal/lsp/cmd/format.go +++ b/internal/lsp/cmd/format.go @@ -19,9 +19,9 @@ import ( // format implements the format verb for gopls. type format struct { - Diff bool `flag:"d" help:"display diffs instead of rewriting files"` - Write bool `flag:"w" help:"write result to (source) file instead of stdout"` - List bool `flag:"l" help:"list files whose formatting differs from gofmt's"` + Diff bool `flag:"d,diff" help:"display diffs instead of rewriting files"` + Write bool `flag:"w,write" help:"write result to (source) file instead of stdout"` + List bool `flag:"l,list" help:"list files whose formatting differs from gofmt's"` app *Application } diff --git a/internal/lsp/cmd/imports.go b/internal/lsp/cmd/imports.go index 405e279b76..215c57f11f 100644 --- a/internal/lsp/cmd/imports.go +++ b/internal/lsp/cmd/imports.go @@ -20,8 +20,8 @@ import ( // imports implements the import verb for gopls. type imports struct { - Diff bool `flag:"d" help:"display diffs instead of rewriting files"` - Write bool `flag:"w" help:"write result to (source) file instead of stdout"` + Diff bool `flag:"d,diff" help:"display diffs instead of rewriting files"` + Write bool `flag:"w,write" help:"write result to (source) file instead of stdout"` app *Application } diff --git a/internal/lsp/cmd/references.go b/internal/lsp/cmd/references.go index d3e1051693..0697d2e11b 100644 --- a/internal/lsp/cmd/references.go +++ b/internal/lsp/cmd/references.go @@ -17,7 +17,7 @@ import ( // references implements the references verb for gopls type references struct { - IncludeDeclaration bool `flag:"d" help:"include the declaration of the specified identifier in the results"` + IncludeDeclaration bool `flag:"d,declaration" help:"include the declaration of the specified identifier in the results"` app *Application } diff --git a/internal/lsp/cmd/rename.go b/internal/lsp/cmd/rename.go index 819106295d..b0a22a1b43 100644 --- a/internal/lsp/cmd/rename.go +++ b/internal/lsp/cmd/rename.go @@ -23,8 +23,8 @@ import ( // rename implements the rename verb for gopls. type rename struct { - Diff bool `flag:"d" help:"display diffs instead of rewriting files"` - Write bool `flag:"w" help:"write result to (source) file instead of stdout"` + Diff bool `flag:"d,diff" help:"display diffs instead of rewriting files"` + Write bool `flag:"w,write" help:"write result to (source) file instead of stdout"` Preserve bool `flag:"preserve" help:"preserve original files"` app *Application diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index 932335457b..df14631da5 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -20,9 +20,9 @@ import ( // suggestedFix implements the fix verb for gopls. type suggestedFix struct { - Diff bool `flag:"d" help:"display diffs instead of rewriting files"` - Write bool `flag:"w" help:"write result to (source) file instead of stdout"` - All bool `flag:"a" help:"apply all fixes, not just preferred fixes"` + Diff bool `flag:"d,diff" help:"display diffs instead of rewriting files"` + Write bool `flag:"w,write" help:"write result to (source) file instead of stdout"` + All bool `flag:"a,all" help:"apply all fixes, not just preferred fixes"` app *Application } diff --git a/internal/lsp/cmd/usage/fix.hlp b/internal/lsp/cmd/usage/fix.hlp index ceba97096b..4789a6c5b3 100644 --- a/internal/lsp/cmd/usage/fix.hlp +++ b/internal/lsp/cmd/usage/fix.hlp @@ -7,6 +7,9 @@ Example: apply suggested fixes for this file $ gopls fix -w internal/lsp/cmd/check.go fix-flags: - -a apply all fixes, not just preferred fixes - -d display diffs instead of rewriting files - -w write result to (source) file instead of stdout + -a,-all + apply all fixes, not just preferred fixes + -d,-diff + display diffs instead of rewriting files + -w,-write + write result to (source) file instead of stdout diff --git a/internal/lsp/cmd/usage/format.hlp b/internal/lsp/cmd/usage/format.hlp index 2fbf1862f9..7ef0bbe431 100644 --- a/internal/lsp/cmd/usage/format.hlp +++ b/internal/lsp/cmd/usage/format.hlp @@ -10,6 +10,9 @@ Example: reformat this file: $ gopls format -w internal/lsp/cmd/check.go format-flags: - -d display diffs instead of rewriting files - -l list files whose formatting differs from gofmt's - -w write result to (source) file instead of stdout + -d,-diff + display diffs instead of rewriting files + -l,-list + list files whose formatting differs from gofmt's + -w,-write + write result to (source) file instead of stdout diff --git a/internal/lsp/cmd/usage/imports.hlp b/internal/lsp/cmd/usage/imports.hlp index 661f64d291..295f4daa2d 100644 --- a/internal/lsp/cmd/usage/imports.hlp +++ b/internal/lsp/cmd/usage/imports.hlp @@ -8,5 +8,7 @@ Example: update imports statements in a file: $ gopls imports -w internal/lsp/cmd/check.go imports-flags: - -d display diffs instead of rewriting files - -w write result to (source) file instead of stdout + -d,-diff + display diffs instead of rewriting files + -w,-write + write result to (source) file instead of stdout diff --git a/internal/lsp/cmd/usage/references.hlp b/internal/lsp/cmd/usage/references.hlp index 34aef38f19..c55ef03370 100644 --- a/internal/lsp/cmd/usage/references.hlp +++ b/internal/lsp/cmd/usage/references.hlp @@ -10,4 +10,5 @@ Example: $ gopls references helper/helper.go:#53 references-flags: - -d include the declaration of the specified identifier in the results + -d,-declaration + include the declaration of the specified identifier in the results diff --git a/internal/lsp/cmd/usage/rename.hlp b/internal/lsp/cmd/usage/rename.hlp index 73446872d4..ae58cbf60a 100644 --- a/internal/lsp/cmd/usage/rename.hlp +++ b/internal/lsp/cmd/usage/rename.hlp @@ -10,7 +10,9 @@ Example: $ gopls rename helper/helper.go:#53 Foo rename-flags: - -d display diffs instead of rewriting files + -d,-diff + display diffs instead of rewriting files -preserve preserve original files - -w write result to (source) file instead of stdout + -w,-write + write result to (source) file instead of stdout diff --git a/internal/lsp/cmd/usage/usage.hlp b/internal/lsp/cmd/usage/usage.hlp index 9288f534b4..b01803b4cb 100644 --- a/internal/lsp/cmd/usage/usage.hlp +++ b/internal/lsp/cmd/usage/usage.hlp @@ -69,6 +69,7 @@ flags: when used with -remote=auto, the -logfile value used to start the daemon -rpc.trace print the full rpc trace in lsp inspector format - -v verbose output - -vv + -v,-verbose + verbose output + -vv,-veryverbose very verbose output diff --git a/internal/tool/tool.go b/internal/tool/tool.go index d4ad8c3ae4..f7c1e42c53 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -15,6 +15,7 @@ import ( "runtime" "runtime/pprof" "runtime/trace" + "strings" "time" ) @@ -28,8 +29,9 @@ import ( // (&Application{}).Main("myapp", "non-flag-command-line-arg-help", os.Args[1:]) // } // It recursively scans the application object for fields with a tag containing -// `flag:"flagname" help:"short help text"`` -// uses all those fields to build command line flags. +// `flag:"flagnames" help:"short help text"`` +// uses all those fields to build command line flags. It will split flagnames on +// commas and add a flag per name. // It expects the Application type to have a method // Run(context.Context, args...string) error // which it invokes only after all command line flag processing has been finished. @@ -168,30 +170,44 @@ func addFlags(f *flag.FlagSet, field reflect.StructField, value reflect.Value) * return nil } // now see if is actually a flag - flagName, isFlag := field.Tag.Lookup("flag") + flagNames, isFlag := field.Tag.Lookup("flag") help := field.Tag.Get("help") - if !isFlag { - // not a flag, but it might be a struct with flags in it - value = resolve(value.Elem()) - if value.Kind() != reflect.Struct { - return nil - } - p, _ := value.Addr().Interface().(*Profile) - // go through all the fields of the struct - for i := 0; i < value.Type().NumField(); i++ { - child := value.Type().Field(i) - v := value.Field(i) - // make sure we have a pointer - if v.Kind() != reflect.Ptr { - v = v.Addr() - } - // check if that field is a flag or contains flags - if fp := addFlags(f, child, v); fp != nil { - p = fp + if isFlag { + nameList := strings.Split(flagNames, ",") + // add the main flag + addFlag(f, value, nameList[0], help) + if len(nameList) > 1 { + // and now add any aliases using the same flag value + fv := f.Lookup(nameList[0]).Value + for _, flagName := range nameList[1:] { + f.Var(fv, flagName, help) } } - return p + return nil } + // not a flag, but it might be a struct with flags in it + value = resolve(value.Elem()) + if value.Kind() != reflect.Struct { + return nil + } + p, _ := value.Addr().Interface().(*Profile) + // go through all the fields of the struct + for i := 0; i < value.Type().NumField(); i++ { + child := value.Type().Field(i) + v := value.Field(i) + // make sure we have a pointer + if v.Kind() != reflect.Ptr { + v = v.Addr() + } + // check if that field is a flag or contains flags + if fp := addFlags(f, child, v); fp != nil { + p = fp + } + } + return p +} + +func addFlag(f *flag.FlagSet, value reflect.Value, flagName string, help string) { switch v := value.Interface().(type) { case flag.Value: f.Var(v, flagName, help) @@ -214,7 +230,6 @@ func addFlags(f *flag.FlagSet, field reflect.StructField, value reflect.Value) * default: log.Fatalf("Cannot understand flag of type %T", v) } - return nil } func resolve(v reflect.Value) reflect.Value {