code review 7132044: cmd/godoc: handle file name arguments better

cmd/godoc: handle file name arguments better

This change prints a warning/suggestion if the user refers to the
directory "foo" when it should be the relative path "./foo".

Furthermore, it discovers if "./foo" resides under an GOPATH or GOROOT
and displays the correct import path. For example,
        cd $GOROOT/src/pkg/archive
        godoc ./zip
will display the docs for "archive/zip" with the correct import path.

Fixes issue 4330.

[adg, 2013-01-16 09:51:36]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[minux1, 2013-01-16 10:58:47]

the behavior for commands seems to be inconsistent with that of normal
packages.

try this:
godoc cmd/godoc # -> command document
cd $GOROOT/src/cmd
godoc godoc # -> package document

godoc archive/zip # -> package document
cd $GOROOT/src/pkg/archive
godoc zip # -> also package document

btw, what do you think it's the best way to say package document for a
command?
i used to do something like this: godoc ./cmd/godoc to view the package
document
for cmd/godoc.

[adg, 2013-01-17 03:15:25]

On 16 January 2013 21:58, minux <minux.ma@gmail.com> wrote:

> the behavior for commands seems to be inconsistent with that of normal
> packages.
>
> try this:
> godoc cmd/godoc # -> command document
> cd $GOROOT/src/cmd
> godoc godoc # -> package document
>
> godoc archive/zip # -> package document
> cd $GOROOT/src/pkg/archive
> godoc zip # -> also package document
>

Ugh! It is so annoying that commands and packages are documented
differently.

I've made some more changes. My goal with this CL was to clean things up a
bit, but I think I've merely maintained the status quo. :/


> btw, what do you think it's the best way to say package document for a
> command?
> i used to do something like this: godoc ./cmd/godoc to view the package
> document
> for cmd/godoc.
>

godoc cmd/godoc
?

[adg, 2013-01-17 05:28:00]

PTAL

This now depends on https://codereview.appspot.com/7129048

[rsc, 2013-01-17 14:12:36]

I am not sure this is a good idea. I am uncomfortable with the
ambiguity this introduces. What if there is a subdirectory called math
and I godoc math? Which one do I get? It has to be the standard math
since otherwise there is no way to name the standard math. But then
people will complain that the subdirectory doesn't work and still have
to learn about ./math. Perhaps we can resolve issue 4330 by making
Rob's example print:

% godoc now  # WHY DOESN'T THIS WORK?
godoc: cannot find import path "now"
For the local directory, use godoc ./now.
%

(The second line would print only if the directory existed.)

Russ

[adg, 2013-01-17 23:25:20]

On 18 January 2013 01:12, Russ Cox <rsc@golang.org> wrote:

> What if there is a subdirectory called math
> and I godoc math? Which one do I get?
>

You get the standard library's math. The order of precedence for
non-absolute paths is: GOROOT, GOPATH, path relative to CWD. This seems the
least surprising order to me. I'm not sure people will complain, as this
change merely gives them a convenience (naming relative paths without
starting them with dot) that they didn't have before.

The other important thing this CL does is make sure the package's import
path is displayed correctly if that package is inside GOROOT or GOPATH.
That's an important improvement.

It's easy enough to alter this change to remove the former functionality
change and add the message you suggest, and keep the latter. What do you
think?

[rsc, 2013-01-18 16:28:44]

> You get the standard library's math. The order of precedence for
> non-absolute paths is: GOROOT, GOPATH, path relative to CWD. This seems the
> least surprising order to me. I'm not sure people will complain, as this
> change merely gives them a convenience (naming relative paths without
> starting them with dot) that they didn't have before.

I am complaining already. :-)

People complaining is probably not a good metric for whether it's a
good design. Convenience today is confusion tomorrow.

> The other important thing this CL does is make sure the package's import
> path is displayed correctly if that package is inside GOROOT or GOPATH.
> That's an important improvement.
>
> It's easy enough to alter this change to remove the former functionality
> change and add the message you suggest, and keep the latter. What do you
> think?

That sounds fine: remove the ambiguity, add a warning if nothing is
found but ./ would fix that, and fix the import path.

Russ

[adg, 2013-01-18 23:05:49]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2013-01-22 21:46:20]

I am happy with where this is going, just want to try to clean up the logic.

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/godoc.go#newco...
src/cmd/godoc/godoc.go:1052: // then it is not usable as command docs.
This is not true. A Go command can be in package main and have docs in package
main. 'Package documentation' is a hack for commands written in C. I don't know
if that affects the surrounding code, but the comment should be more precise.

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/main.go
File src/cmd/godoc/main.go (right):

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/main.go#newcod...
src/cmd/godoc/main.go:384: // If command docs are present, choose them above all
else.
This is different from the way it used to work.
It used to be that package wins unless you say cmd/.

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/main.go#newcod...
src/cmd/godoc/main.go:406: fmt.Printf("use 'godoc %s%s' for documentation on the
%s command \n\n", cmdPrefix, relpath, relpath)
How does this work anymore without forceCmd?
I'm pretty lost at this point in the code. Can you make this clearer?

[adg, 2013-01-23 05:58:29]

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/godoc.go#newco...
src/cmd/godoc/godoc.go:1052: // then it is not usable as command docs.
On 2013/01/22 21:46:20, rsc wrote:
> This is not true. A Go command can be in package main and have docs in package
> main. 'Package documentation' is a hack for commands written in C. I don't
know
> if that affects the surrounding code, but the comment should be more precise.

If isPkg is false then we're looking for doc.go with "package documentation". I
updated the comment, hopefully it is more clear.

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/main.go
File src/cmd/godoc/main.go (right):

https://codereview.appspot.com/7132044/diff/5004/src/cmd/godoc/main.go#newcod...
src/cmd/godoc/main.go:384: // If command docs are present, choose them above all
else.
On 2013/01/22 21:46:20, rsc wrote:
> This is different from the way it used to work.
> It used to be that package wins unless you say cmd/.

I think we need to nail down when to use command docs (with cmdHandler, which
does not show any type, func, var, or const declarations) and when to use
package docs (with pkgHandler).

Proposal:

Show command docs when:
a. there is a doc.go in the directory with a "package documentation" clause
b. the files in the directory belong to "package main"
Otherwise show package docs.

I think point b is not the case at the moment.

[gobot, 2013-12-20 16:21:40]

Replacing golang-dev with golang-codereviews.

[rsc, 2014-03-05 19:34:03]

R=close

these files don't exist anymore