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/godoc: No command name if package main is in "src" #22878

Open
rivo opened this issue Nov 25, 2017 · 13 comments

Comments

@rivo
Copy link

commented Nov 25, 2017

What version of Go are you using (go version)?

go version go1.9.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

  • Create a project with package main in the "src" directory, e.g. "src/main.go":
package main

func main() {
}
  • Run Godoc with godoc -http=:6060
  • Open http://localhost:6060/pkg/
  • Check out the top of the page.

What did you expect to see?

The page title should say "Command main."

What did you see instead?

It says "Command ."

If the "main" package is in a subdirectory of "src", e.g. "src/main/main.go", it will say "Command main". I typically keep the main function of my application in the "src" directory (which I don't think is very uncommon). The nice thing is also that the main package page will show my application's documentation (that of package "main"). But Godoc will not recognize the command name.

I checked the code. It takes the command name from the page URL (minus the "pkg"). In this case, it's an empty URL so it ends up with an empty string. I think it makes sense to handle this special case and use the string "main" (this applies only if "IsMain" is true anyway).

Thoughts?

Edit: By "src", I mean "$GOPATH/src".

@gopherbot gopherbot added this to the Unreleased milestone Nov 25, 2017

@cznic

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2017

By 'src', do you mean $GOPATH/src'?

@rivo

This comment has been minimized.

Copy link
Author

commented Nov 25, 2017

Yes, sorry. I added that.

@cznic

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2017

Well, then it works as intended. The import path of a package may not be empty.

@rivo

This comment has been minimized.

Copy link
Author

commented Nov 25, 2017

I'm not sure I understand:

  • The "main" package in "$GOPATH/src" is not imported anywhere so there is no "import path".
  • You seem to to imply that the "$GOPATH/src" directory must be empty. Go accepts and works nicely with code in that directory. I also didn't find any documentation that states that there must be no code. Can you point me to such a requirement?
  • I would argue that displaying "Command ." is not intended as it's rather confusing. On a side note, for a package "abc/xyz/main", the header will say "Command main." (It's not the import path that's displayed.)

Maybe I'm missing something?

@mvdan

This comment has been minimized.

Copy link
Member

commented Nov 25, 2017

The "main" package in "$GOPATH/src" is not imported anywhere so there is no "import path".

All packages inside GOPATH have a package path, even if they are never imported.

Also, note that the Command <x> part doesn't use the package name, but the last element of the package path. If you create a main package in $GOPATH/src/foo, it will show Command foo. That's also the name that people will go get and use as a binary, which is why it's useful. Otherwise all main package doc pages would seem the same.

I also didn't find any documentation that states that there must be no code. Can you point me to such a requirement?

I don't recall such an explicit piece of the docs, but it's very rare for people to put code there. It's not importable, it can't be used with go get, and it has none (or a confusing?) package path. I'm not sure why it would ever be useful.

@rivo

This comment has been minimized.

Copy link
Author

commented Nov 26, 2017

Thanks for the explanation. I understand that it's not recommended to put code in $GOPATH/src for the reasons you mentioned. The code I put there is not meant to be imported anywhere. It's simply the main() function of my application (plus some general application documentation).

Maybe I'm just confused that Go as well as Godoc work well with that scenario (except for the point raised in this issue). In many other places, Go throws compile errors for things that are not recommended. Maybe "Package path must not be empty" would be a reasonable message. But it may break a lot of projects that do what I'm doing here. And these projects will always see a weird page title "Command ."

It's not a huge issue so feel free to close it if you feel this case should not be handled. Personally, I think a small addition here could do the trick:

if tabtitle == "" {
    tabtitle = "main"
}
@cznic

This comment has been minimized.

Copy link
Contributor

commented Nov 26, 2017

Instead of twisting the Go tool to support things it does not support for good reasons just mkdir $GOPATH/src/tmp and put any throwaway code there.

@mvdan

This comment has been minimized.

Copy link
Member

commented Nov 26, 2017

Personally, I think a small addition here could do the trick:

I don't think that would be a good change to do - the package name could be something other than main. A non-main package in that directory makes even less sense as it's completely useless, but hard-coding main like this could lead to even more confusion.

I think falling back to the package name when the package path is . could be easily as confusing, to be honest. It would break the consistency of what Command <x> means.

I'm not going to close this issue myself, because I'm not certain as to what degree the "root" package path is supported and supposed to work. Hopefully someone can come with a link that clarifies things one way or another. Otherwise, I'm not convinced that there's anything to fix in godoc here.

@rivo

This comment has been minimized.

Copy link
Author

commented Nov 26, 2017

Actually, the package name is main, see here:

if info.IsMain {
	// assume that the directory name is the command name
	_, tabtitle = pathpkg.Split(relpath)
	title = "Command "
} else {
	title = "Package "
}

Further up, you will find:

info.IsMain = pkgname == "main"
@mvdan

This comment has been minimized.

Copy link
Member

commented Nov 26, 2017

Ah, my bad for misreading the code. The second part of my reply still stands, though.

@agnivade

This comment has been minimized.

Copy link
Member

commented Mar 24, 2018

This is kind of an edge case with no clear solution. While the Go docs does not explicitly forbid code inside the $GOPATH/src directory, it does mention to make a folder inside the workspace.

I would go further to say that the word "Command" is itself misleading for the package root at $GOPATH/src. Because "command" would mean that it is importing and running some other package. However, you can have as many main packages inside $GOPATH/src as you like, each doing its own separate thing. So to me, both . and main are equally confusing.

I would prefer to rename the title to "Root" or something similar. But keeping in mind that the docs suggest to create a folder, it might just be better to leave it as is and close the issue.

@rivo

This comment has been minimized.

Copy link
Author

commented Mar 25, 2018

I like the idea of displaying "Root". Eventually, someone who hasn't seen the recommendation will wonder about it again. I don't see the harm in making it work for that edge case.

@agnivade

This comment has been minimized.

Copy link
Member

commented Mar 25, 2018

I have no opinions either way. Somebody else will have to take a call. Needs a NeedsDecision label.

@mvdan mvdan added the NeedsDecision label Mar 25, 2018

@gopherbot gopherbot added the Tools label Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.