cmd/go: better UI for go doc

Print it out much like godoc so there isn't a single block of text.
Print the symbol before its comment and indent the comment so
individual symbols separate visually.

Buffer the output.

Add a -c option to force case-sensitive matching.

Allow two arguments, like godoc, to help disambiguate cases
where path and symbol may be confused.

Improve the documentation printed by go help doc.

Change-Id: If687aad04bbacdf7dbe4bf7636de9fe96f756fd0
Reviewed-on: https://go-review.googlesource.com/9471
Reviewed-by: Russ Cox <rsc@golang.org>

2 files changed, 126 insertions, 64 deletions

diff --git a/src/cmd/doc/main.go b/src/cmd/doc/main.go
index e0178effce..22694287e9 100644
--- a/src/cmd/doc/main.go
+++ b/src/cmd/doc/main.go
@@ -2,14 +2,26 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Doc (usually run as go doc) accepts zero or one argument, interpreted as:
+// Doc (usually run as go doc) accepts zero, one or two arguments.
+//
+// Zero arguments:
 //	go doc
+// Show the documentation for the package in the current directory.
+//
+// One argument:
 //	go doc <pkg>
 //	go doc <sym>[.<method>]
 //	go doc [<pkg>].<sym>[.<method>]
 // The first item in this list that succeeds is the one whose documentation
-// is printed. If there is no argument, the package in the current directory
-// is chosen.
+// is printed. If there is a symbol but no package, the package in the current
+// directory is chosen.
+//
+// Two arguments:
+//	go doc <pkg> <sym>[.<method>]
+//
+// Show the documentation for the package, symbol, and method. The
+// first argument must be a full package path. This is similar to the
+// command-line usage for the godoc command.
 //
 // For complete documentation, run "go help doc".
 package main
@@ -28,14 +40,10 @@ import (
 )
 
 var (
-	unexported bool
+	unexported = flag.Bool("u", false, "show unexported symbols as well as exported")
+	matchCase  = flag.Bool("c", false, "symbol matching honors case (paths not affected)")
 )
 
-func init() {
-	flag.BoolVar(&unexported, "unexported", false, "show unexported symbols as well as exported")
-	flag.BoolVar(&unexported, "u", false, "shorthand for -unexported")
-}
-
 // usage is a replacement usage function for the flags package.
 func usage() {
 	fmt.Fprintf(os.Stderr, "Usage of [go] doc:\n")
@@ -43,6 +51,7 @@ func usage() {
 	fmt.Fprintf(os.Stderr, "\tgo doc <pkg>\n")
 	fmt.Fprintf(os.Stderr, "\tgo doc <sym>[.<method>]\n")
 	fmt.Fprintf(os.Stderr, "\tgo doc [<pkg>].<sym>[.<method>]\n")
+	fmt.Fprintf(os.Stderr, "\tgo doc <pkg> <sym>[.<method>]\n")
 	fmt.Fprintf(os.Stderr, "For more information run\n")
 	fmt.Fprintf(os.Stderr, "\tgo help doc\n\n")
 	fmt.Fprintf(os.Stderr, "Flags:\n")
@@ -55,9 +64,9 @@ func main() {
 	log.SetPrefix("doc: ")
 	flag.Usage = usage
 	flag.Parse()
-	buildPackage, symbol := parseArg()
+	buildPackage, userPath, symbol := parseArgs()
 	symbol, method := parseSymbol(symbol)
-	pkg := parsePackage(buildPackage)
+	pkg := parsePackage(buildPackage, userPath)
 	switch {
 	case symbol == "":
 		pkg.packageDoc()
@@ -69,18 +78,27 @@ func main() {
 	}
 }
 
-// parseArg analyzes the argument (if any) and returns the package
-// it represents and the symbol (possibly with a .method) within that
-// package. parseSymbol is used to analyze the symbol itself.
-func parseArg() (*build.Package, string) {
+// parseArgs analyzes the arguments (if any) and returns the package
+// it represents, the part of the argument the user used to identify
+// the path (or "" if it's the current package) and the symbol
+// (possibly with a .method) within that package.
+// parseSymbol is used to analyze the symbol itself.
+func parseArgs() (*build.Package, string, string) {
 	switch flag.NArg() {
 	default:
 		usage()
 	case 0:
 		// Easy: current directory.
-		return importDir("."), ""
+		return importDir("."), "", ""
 	case 1:
 		// Done below.
+	case 2:
+		// Package must be importable.
+		pkg, err := build.Import(flag.Arg(0), "", build.ImportComment)
+		if err != nil {
+			log.Fatal(err)
+		}
+		return pkg, flag.Arg(0), flag.Arg(1)
 	}
 	// Usual case: one argument.
 	arg := flag.Arg(0)
@@ -90,17 +108,16 @@ func parseArg() (*build.Package, string) {
 	// package paths as their prefix.
 	pkg, err := build.Import(arg, "", build.ImportComment)
 	if err == nil {
-		return pkg, ""
+		return pkg, arg, ""
 	}
 	// Another disambiguator: If the symbol starts with an upper
 	// case letter, it can only be a symbol in the current directory.
 	// Kills the problem caused by case-insensitive file systems
 	// matching an upper case name as a package name.
 	if isUpper(arg) {
-		println("HERE", arg)
 		pkg, err := build.ImportDir(".", build.ImportComment)
 		if err == nil {
-			return pkg, arg
+			return pkg, "", arg
 		}
 	}
 	// If it has a slash, it must be a package path but there is a symbol.
@@ -125,16 +142,13 @@ func parseArg() (*build.Package, string) {
 		// Have we identified a package already?
 		pkg, err := build.Import(arg[0:period], "", build.ImportComment)
 		if err == nil {
-			return pkg, symbol
+			return pkg, arg[0:period], symbol
 		}
 		// See if we have the basename or tail of a package, as in json for encoding/json
 		// or ivy/value for robpike.io/ivy/value.
 		path := findPackage(arg[0:period])
 		if path != "" {
-			return importDir(path), symbol
-		}
-		if path != "" {
-			return importDir(path), symbol
+			return importDir(path), arg[0:period], symbol
 		}
 	}
 	// If it has a slash, we've failed.
@@ -142,7 +156,7 @@ func parseArg() (*build.Package, string) {
 		log.Fatalf("no such package %s", arg[0:period])
 	}
 	// Guess it's a symbol in the current directory.
-	return importDir("."), arg
+	return importDir("."), "", arg
 }
 
 // importDir is just an error-catching wrapper for build.ImportDir.
@@ -194,7 +208,7 @@ func isIdentifier(name string) {
 // If the unexported flag (-u) is true, isExported returns true because
 // it means that we treat the name as if it is exported.
 func isExported(name string) bool {
-	return unexported || isUpper(name)
+	return *unexported || isUpper(name)
 }
 
 // isUpper reports whether the name starts with an upper case letter.
diff --git a/src/cmd/doc/pkg.go b/src/cmd/doc/pkg.go
index e08b756239..580a91f7e6 100644
--- a/src/cmd/doc/pkg.go
+++ b/src/cmd/doc/pkg.go
@@ -20,17 +20,19 @@ import (
 )
 
 type Package struct {
-	name  string       // Package name, json for encoding/json.
-	pkg   *ast.Package // Parsed package.
-	file  *ast.File    // Merged from all files in the package
-	doc   *doc.Package
-	build *build.Package
-	fs    *token.FileSet // Needed for printing.
+	name     string       // Package name, json for encoding/json.
+	userPath string       // String the user used to find this package.
+	pkg      *ast.Package // Parsed package.
+	file     *ast.File    // Merged from all files in the package
+	doc      *doc.Package
+	build    *build.Package
+	fs       *token.FileSet // Needed for printing.
+	buf      bytes.Buffer
 }
 
 // parsePackage turns the build package we found into a parsed package
 // we can then use to generate documentation.
-func parsePackage(pkg *build.Package) *Package {
+func parsePackage(pkg *build.Package, userPath string) *Package {
 	fs := token.NewFileSet()
 	// include tells parser.ParseDir which files to include.
 	// That means the file must be in the build package's GoFiles or CgoFiles
@@ -73,35 +75,54 @@ func parsePackage(pkg *build.Package) *Package {
 	}
 
 	return &Package{
-		name:  pkg.Name,
-		pkg:   astPkg,
-		file:  ast.MergePackageFiles(astPkg, 0),
-		doc:   docPkg,
-		build: pkg,
-		fs:    fs,
+		name:     pkg.Name,
+		userPath: userPath,
+		pkg:      astPkg,
+		file:     ast.MergePackageFiles(astPkg, 0),
+		doc:      docPkg,
+		build:    pkg,
+		fs:       fs,
 	}
 }
 
-var formatBuf bytes.Buffer // One instance to minimize allocation. TODO: Buffer all output.
+func (pkg *Package) Printf(format string, args ...interface{}) {
+	fmt.Fprintf(&pkg.buf, format, args...)
+}
+
+func (pkg *Package) flush() {
+	_, err := os.Stdout.Write(pkg.buf.Bytes())
+	if err != nil {
+		log.Fatal(err)
+	}
+	pkg.buf.Reset() // Not needed, but it's a flush.
+}
+
+var newlineBytes = []byte("\n\n") // We never ask for more than 2.
+
+// newlines guarantees there are n newlines at the end of the buffer.
+func (pkg *Package) newlines(n int) {
+	for !bytes.HasSuffix(pkg.buf.Bytes(), newlineBytes[:n]) {
+		pkg.buf.WriteRune('\n')
+	}
+}
 
 // emit prints the node.
 func (pkg *Package) emit(comment string, node ast.Node) {
 	if node != nil {
-		formatBuf.Reset()
-		if comment != "" {
-			doc.ToText(&formatBuf, comment, "", "\t", 80)
-		}
-		err := format.Node(&formatBuf, pkg.fs, node)
+		err := format.Node(&pkg.buf, pkg.fs, node)
 		if err != nil {
 			log.Fatal(err)
 		}
-		if formatBuf.Len() > 0 && formatBuf.Bytes()[formatBuf.Len()-1] != '\n' {
-			formatBuf.WriteRune('\n')
+		if comment != "" {
+			pkg.newlines(1)
+			doc.ToText(&pkg.buf, comment, "    ", "\t", 80)
 		}
-		os.Stdout.Write(formatBuf.Bytes())
+		pkg.newlines(1)
 	}
 }
 
+var formatBuf bytes.Buffer // Reusable to avoid allocation.
+
 // formatNode is a helper function for printing.
 func (pkg *Package) formatNode(node ast.Node) []byte {
 	formatBuf.Reset()
@@ -137,7 +158,7 @@ func (pkg *Package) oneLineValueGenDecl(decl *ast.GenDecl) {
 		if i < len(valueSpec.Values) && valueSpec.Values[i] != nil {
 			val = fmt.Sprintf(" = %s", pkg.formatNode(valueSpec.Values[i]))
 		}
-		fmt.Printf("%s %s%s%s%s\n", decl.Tok, valueSpec.Names[0], typ, val, dotDotDot)
+		pkg.Printf("%s %s%s%s%s\n", decl.Tok, valueSpec.Names[0], typ, val, dotDotDot)
 		break
 	}
 }
@@ -148,33 +169,46 @@ func (pkg *Package) oneLineTypeDecl(spec *ast.TypeSpec) {
 	spec.Comment = nil
 	switch spec.Type.(type) {
 	case *ast.InterfaceType:
-		fmt.Printf("type %s interface { ... }\n", spec.Name)
+		pkg.Printf("type %s interface { ... }\n", spec.Name)
 	case *ast.StructType:
-		fmt.Printf("type %s struct { ... }\n", spec.Name)
+		pkg.Printf("type %s struct { ... }\n", spec.Name)
 	default:
-		fmt.Printf("type %s %s\n", spec.Name, pkg.formatNode(spec.Type))
+		pkg.Printf("type %s %s\n", spec.Name, pkg.formatNode(spec.Type))
 	}
 }
 
 // packageDoc prints the docs for the package (package doc plus one-liners of the rest).
-// TODO: Sort the output.
 func (pkg *Package) packageDoc() {
-	// Package comment.
+	defer pkg.flush()
+	pkg.packageClause(false)
+
+	doc.ToText(&pkg.buf, pkg.doc.Doc, "", "\t", 80)
+	pkg.newlines(2)
+
+	pkg.valueSummary(pkg.doc.Consts)
+	pkg.valueSummary(pkg.doc.Vars)
+	pkg.funcSummary(pkg.doc.Funcs)
+	pkg.typeSummary()
+}
+
+// packageClause prints the package clause.
+// The argument boolean, if true, suppresses the output if the
+// user's argument is identical to the actual package path or
+// is empty, meaning it's the current directory.
+func (pkg *Package) packageClause(checkUserPath bool) {
+	if checkUserPath {
+		if pkg.userPath == "" || pkg.userPath == pkg.build.ImportPath {
+			return
+		}
+	}
 	importPath := pkg.build.ImportComment
 	if importPath == "" {
 		importPath = pkg.build.ImportPath
 	}
-	fmt.Printf("package %s // import %q\n\n", pkg.name, importPath)
+	pkg.Printf("package %s // import %q\n\n", pkg.name, importPath)
 	if importPath != pkg.build.ImportPath {
-		fmt.Printf("WARNING: package source is installed in %q\n", pkg.build.ImportPath)
+		pkg.Printf("WARNING: package source is installed in %q\n", pkg.build.ImportPath)
 	}
-	doc.ToText(os.Stdout, pkg.doc.Doc, "", "\t", 80)
-	fmt.Print("\n")
-
-	pkg.valueSummary(pkg.doc.Consts)
-	pkg.valueSummary(pkg.doc.Vars)
-	pkg.funcSummary(pkg.doc.Funcs)
-	pkg.typeSummary()
 }
 
 // valueSummary prints a one-line summary for each set of values and constants.
@@ -265,9 +299,13 @@ func (pkg *Package) findTypeSpec(decl *ast.GenDecl, symbol string) *ast.TypeSpec
 // symbolDoc prints the docs for symbol. There may be multiple matches.
 // If symbol matches a type, output includes its methods factories and associated constants.
 func (pkg *Package) symbolDoc(symbol string) {
+	defer pkg.flush()
 	found := false
 	// Functions.
 	for _, fun := range pkg.findFuncs(symbol) {
+		if !found {
+			pkg.packageClause(true)
+		}
 		// Symbol is a function.
 		decl := fun.Decl
 		decl.Body = nil
@@ -278,11 +316,17 @@ func (pkg *Package) symbolDoc(symbol string) {
 	values := pkg.findValues(symbol, pkg.doc.Consts)
 	values = append(values, pkg.findValues(symbol, pkg.doc.Vars)...)
 	for _, value := range values {
+		if !found {
+			pkg.packageClause(true)
+		}
 		pkg.emit(value.Doc, value.Decl)
 		found = true
 	}
 	// Types.
 	for _, typ := range pkg.findTypes(symbol) {
+		if !found {
+			pkg.packageClause(true)
+		}
 		decl := typ.Decl
 		spec := pkg.findTypeSpec(decl, typ.Name)
 		trimUnexportedFields(spec)
@@ -306,7 +350,7 @@ func (pkg *Package) symbolDoc(symbol string) {
 // trimUnexportedFields modifies spec in place to elide unexported fields (unless
 // the unexported flag is set). If spec is not a structure declartion, nothing happens.
 func trimUnexportedFields(spec *ast.TypeSpec) {
-	if unexported {
+	if *unexported {
 		// We're printing all fields.
 		return
 	}
@@ -349,6 +393,7 @@ func trimUnexportedFields(spec *ast.TypeSpec) {
 
 // methodDoc prints the docs for matches of symbol.method.
 func (pkg *Package) methodDoc(symbol, method string) {
+	defer pkg.flush()
 	types := pkg.findTypes(symbol)
 	if types == nil {
 		log.Fatalf("symbol %s is not a type in package %s installed in %q", symbol, pkg.name, pkg.build.ImportPath)
@@ -376,6 +421,9 @@ func match(user, program string) bool {
 	if !isExported(program) {
 		return false
 	}
+	if *matchCase {
+		return user == program
+	}
 	for _, u := range user {
 		p, w := utf8.DecodeRuneInString(program)
 		program = program[w:]