Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: improve help output #35855

Open
cespare opened this issue Nov 26, 2019 · 9 comments
Open

x/tools/gopls: improve help output #35855

cespare opened this issue Nov 26, 2019 · 9 comments

Comments

@cespare
Copy link
Contributor

@cespare cespare commented Nov 26, 2019

I installed golang.org/x/tools/gopls@latest and tried to look at its command-line tool options (rename, etc).

First I tried -h, since this is a typical way to make Go tools show usage:

$ gopls -h
Usage of gopls:
  -debug string
        Serve debug information on the supplied address
  -listen string
        address on which to listen for remote connections
  -logfile string
        filename to log to. if value is "auto", then logging to a default output file is enabled
  -mode string
        no effect
  -ocagent string
        The address of the ocagent, or off (default "off")
  -port int
        port on which to run gopls for debugging purposes
  -profile.cpu string
        write CPU profile to this file
  -profile.mem string
        write memory profile to this file
  -profile.trace string
        write trace log to this file
  -remote string
        *EXPERIMENTAL* - forward all commands to a remote lsp
  -rpc.trace
        Print the full rpc trace in lsp inspector format
  -v    Verbose output

So this shows the flags (for which command?), but not the actual commands. Next I tried gopls help:

$ gopls help
gopls: Unknown command help
The Go Language source tools.

Usage: gopls [flags] <command> [command-flags] [command-args]

Available commands are:
  serve : run a server for Go code using the Language Server Protocol
  bug : report a bug in gopls
  check : show diagnostic results for the specified file
  format : format the code according to the go standard
  links : list links in a file
  imports : updates import statements
  query : answer queries about go source code
  references : display selected identifier's references
  rename : rename selected identifier
  signature : display selected identifier's signature
  fix : apply suggested fixes
  symbols : display selected file's symbols
  version : print the gopls version information

gopls flags are:

That works, but only accidentally ("Unknown command help"). Also note the trailing "gopls flags are:" followed by no flags.

Finally, I tried running gopls rename -h to see how one uses this tool:

$ gopls rename -h
Usage of rename:
  -d    display diffs instead of rewriting files
  -w    write result to (source) file instead of stdout

This does not provide enough information to actually use gopls rename.

We should improve this situation:

  • gopls -h should print out the possible commands.
  • After printing the commands, it should print the common flags (as well as some text explaining that these are flags that apply across all commands), and perhaps some text suggesting the user can run gopls <command> -h to find the flags for a particular command.
  • Each command's -h output could say what the command actually does and how to use it, not just list the flags.

For bonus points:

@gopherbot gopherbot added this to the Unreleased milestone Nov 26, 2019
@stamblerre stamblerre self-assigned this Nov 26, 2019
@stamblerre stamblerre removed their assignment Nov 30, 2019
@jbszczepaniak

This comment has been minimized.

Copy link

@jbszczepaniak jbszczepaniak commented Nov 30, 2019

I'll work on this one.

@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Dec 4, 2019
@jbszczepaniak

This comment has been minimized.

Copy link

@jbszczepaniak jbszczepaniak commented Dec 4, 2019

@stamblerre Currently all of the commands are printed as a signle list:

Available commands are:
  serve : run a server for Go code using the Language Server Protocol
  bug : report a bug in gopls
  check : show diagnostic results for the specified file
  folding_ranges : display selected file's folding ranges
  format : format the code according to the go standard
  highlight : display selected identifier's highlights
  implementation : display selected identifier's implementation
  imports : updates import statements
  links : list links in a file
  query : answer queries about go source code
  references : display selected identifier's references
  rename : rename selected identifier
  signature : display selected identifier's signature
  fix : apply suggested fixes
  symbols : display selected file's symbols
  version : print the gopls version information

What do you think about separating this into "main" (looking for better naming) commands and "features" that can be run adhoc like this:

Available commands are:
main:
  serve : run a server for Go code using the Language Server Protocol
  bug : report a bug in gopls
  version : print the gopls version information
features:
  check : show diagnostic results for the specified file
  folding_ranges : display selected file's folding ranges
  format : format the code according to the go standard
  highlight : display selected identifier's highlights
  implementation : display selected identifier's implementation
  imports : updates import statements
  links : list links in a file
  query : answer queries about go source code
  references : display selected identifier's references
  rename : rename selected identifier
  signature : display selected identifier's signature
  fix : apply suggested fixes
  symbols : display selected file's symbols

I think it shows intent of the commands more clearly.

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Dec 4, 2019

@jbszczepaniak: That sounds reasonable to me!

@jbszczepaniak

This comment has been minimized.

Copy link

@jbszczepaniak jbszczepaniak commented Dec 7, 2019

OK, one more question. Should running command without any flag nor command behave like this? I find it confusing:

➜  gopls git:(master) ✗ gopls

gopls: missing Content-Length header

If you'll not provide valid json rpc message it does nothing and when you hit enter you get missing content message. My proposition is to print help whenever you run just gopls and allow feeding stdin with json under additional command. What do you think?

@jbszczepaniak

This comment has been minimized.

Copy link

@jbszczepaniak jbszczepaniak commented Dec 7, 2019

And one more again ;)
Following flags printed when doing gopls -h:

  -debug string
    	Serve debug information on the supplied address
  -listen string
    	address on which to listen for remote connections
  -logfile string
    	filename to log to. if value is "auto", then logging to a default output file is enabled
  -mode string
    	no effect
  -port int
    	port on which to run gopls for debugging purposes
  -rpc.trace
    	Print the full rpc trace in lsp inspector format

concern only gopls serve command. Shouldn't they be printed only with gopls serve -h? I don't think it makes sense to print them on gopls -h

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Dec 7, 2019

@jbszczepaniak: Both of your comments sound good! We can also have more discussion once you send out a CL for review.

@jbszczepaniak

This comment has been minimized.

Copy link

@jbszczepaniak jbszczepaniak commented Dec 7, 2019

@stamblerre could you tell me about this compatibility thing?
https://github.com/golang/tools/blob/master/internal/lsp/cmd/cmd.go#L117

// If no arguments are passed it will invoke the server sub command, as a
// temporary measure for compatibility.

It pops here as well https://github.com/golang/tools/blob/master/internal/lsp/cmd/cmd.go#L43:

// We include the server configuration directly for now, so the flags work
// even without the verb.
// TODO: Remove this when we stop allowing the serve verb by default.
Serve Serve

With this my proposal cannot be done and probably I am missing some context and discussion that took place some time ago.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 8, 2019

Change https://golang.org/cl/210359 mentions this issue: internal/lsp: clean up command line of gopls

@jbszczepaniak

This comment has been minimized.

Copy link

@jbszczepaniak jbszczepaniak commented Dec 8, 2019

I decided to submit first change for this issue to keep things small. Things I didn't do:

  • update README
  • improvment of usage of each subcommands
  • removing serve as default command
  • help is not an alias to --help
gopherbot pushed a commit to golang/tools that referenced this issue Dec 16, 2019
Descriptions of certain commands were changed to not start with capital
letter.

All of the commands were splitted into so called main
commands and feature commands.

Package tool did have a limitation that revealed itself when command
was invoked only with `-h`, i.e. `gopls -h`. Limitation was that in
above mentioned case, FlagSet.Parse() was intercepting `-h` flag and
printing just default usage.

Updates golang/go#35855

Change-Id: I9bd27fc72e8fb8d18025d95ebcae974dd5583e91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210359
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.