[go] cmd/go/internal/doc: support @version suffix on first argument
3 views
Skip to first unread message
Ian Alexander (Gerrit)
Feb 20, 2026, 3:18:49 PM (7 days ago) Feb 20
to goph...@pubsubhelper.golang.org, Michael Matloob, Go LUCI, golang-co...@googlegroups.com
Attention needed from Michael Matloob
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Michael Matloob
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Feb 23, 2026, 12:57:14 PM (4 days ago) Feb 23
to Ian Alexander, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Ian Alexander
Michael Matloob voted and added 5 comments![]( "Open in Gerrit")
Votes added by Michael Matloob
| Code-Review | +2 |
5 comments
File src/cmd/go/internal/doc/doc.go
Line 381, Patchset 6 (Latest):
if version != "" && (isDotSlash(arg) || filepath.IsAbs(arg)) {Michael Matloob . unresolved
This is an interesting case... elsewhere we use `build.IsLocalImport` for this check but here we also check for `.\` and `..\` which would also be local imports. I think we should be consistent with the rest of the code so we should also call IsLocalImport here but arguably we should do a `filepath.ToSlash` before we call `build.IsLocalImport` so we still catch the `.\` and `..\` cases. (and we should probably do the same in the other places we are checking for `pkg@version`.
File src/cmd/go/internal/doc/doc_test.go
Line 914, Patchset 6 (Latest):
err := do(context.Background(), &b, &flagSet, test.args)Michael Matloob . unresolved
use `t.Context()`?
File src/cmd/go/internal/doc/mod.go
Line 25, Patchset 6 (Latest):
noneSelected := func(path string) (version string) { return "none" }
qrs, err := modload.QueryPackages(loaderState, ctx, pkgPath, version, noneSelected, nil)
if err != nil {
return nil, err
}
qr := qrs[0]
dir, err := loaderState.Fetcher().Download(ctx, qr.Mod)
if err != nil {
return nil, err
}
// Calculate the directory for the package within the module.
if len(qr.Packages) > 0 && qr.Packages[0] != qr.Mod.Path {
rel := strings.TrimPrefix(qr.Packages[0], qr.Mod.Path+"/")
dir = filepath.Join(dir, filepath.FromSlash(rel))
}
return buildCtx.ImportDir(dir, build.ImportComment)Michael Matloob . unresolved
Can we use `load.PackagesAndErrorsOutsideModule` and return the package? (If `p` is a `*load.Package` returned by `PackagesAndErrorsOutsideModule` you can get to the build.Package using `p.Internal.Build`.) Ideally in the rest of go doc we just use the standard loader logic instead of using build.Packages directly
File src/cmd/go/testdata/script/issue_63696.txt
Line 3, Patchset 6 (Latest):
go mod tidyMichael Matloob . resolved
optional: I think we can remove this line?
(we could also run `go get` without any arguments and it should pull in the requirements of the package in the current directory, and will pull in rsc.io/quote because of the import)
Line 4, Patchset 6 (Latest):
go buildMichael Matloob . unresolved
can we remove this? a build would slow down the test.
Related details
Attention is currently required from:
- Ian Alexander
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Feb 23, 2026, 1:04:22 PM (4 days ago) Feb 23
to Ian Alexander, goph...@pubsubhelper.golang.org, Michael Matloob, Go LUCI, golang-co...@googlegroups.com
Attention needed from Ian Alexander
Michael Matloob voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Ian Alexander
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Feb 26, 2026, 8:44:09 PM (22 hours ago) Feb 26
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Ian Alexander
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
Ian Alexander uploaded patch set #7 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Ian Alexander
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Feb 26, 2026, 8:47:48 PM (22 hours ago) Feb 26
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Ian Alexander
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
Ian Alexander uploaded patch set #8 to this change.
Related details
Attention is currently required from:
- Ian Alexander
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Feb 26, 2026, 8:48:44 PM (22 hours ago) Feb 26
to goph...@pubsubhelper.golang.org, Michael Matloob, Michael Matloob, Go LUCI, golang-co...@googlegroups.com
Ian Alexander voted and added 4 comments![]( "Open in Gerrit")
Votes added by Ian Alexander
| Commit-Queue | +1 |
4 comments
File src/cmd/go/internal/doc/doc.go
Line 381, Patchset 6:
if version != "" && (isDotSlash(arg) || filepath.IsAbs(arg)) {Michael Matloob . resolved
This is an interesting case... elsewhere we use `build.IsLocalImport` for this check but here we also check for `.\` and `..\` which would also be local imports. I think we should be consistent with the rest of the code so we should also call IsLocalImport here but arguably we should do a `filepath.ToSlash` before we call `build.IsLocalImport` so we still catch the `.\` and `..\` cases. (and we should probably do the same in the other places we are checking for `pkg@version`.
Ian Alexander
Done
File src/cmd/go/internal/doc/doc_test.go
Line 914, Patchset 6:
err := do(context.Background(), &b, &flagSet, test.args)Michael Matloob . resolved
Ian Alexanderuse `t.Context()`?
Done
File src/cmd/go/internal/doc/mod.go
Line 25, Patchset 6:
noneSelected := func(path string) (version string) { return "none" }
qrs, err := modload.QueryPackages(loaderState, ctx, pkgPath, version, noneSelected, nil)
if err != nil {
return nil, err
}
qr := qrs[0]
dir, err := loaderState.Fetcher().Download(ctx, qr.Mod)
if err != nil {
return nil, err
}
// Calculate the directory for the package within the module.
if len(qr.Packages) > 0 && qr.Packages[0] != qr.Mod.Path {
rel := strings.TrimPrefix(qr.Packages[0], qr.Mod.Path+"/")
dir = filepath.Join(dir, filepath.FromSlash(rel))
}
return buildCtx.ImportDir(dir, build.ImportComment)Michael Matloob . resolved
Can we use `load.PackagesAndErrorsOutsideModule` and return the package? (If `p` is a `*load.Package` returned by `PackagesAndErrorsOutsideModule` you can get to the build.Package using `p.Internal.Build`.) Ideally in the rest of go doc we just use the standard loader logic instead of using build.Packages directly
Ian Alexander
Done
File src/cmd/go/testdata/script/issue_63696.txt
can we remove this? a build would slow down the test.
Ian Alexander
Done
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Feb 26, 2026, 9:36:39 PM (21 hours ago) Feb 26
to goph...@pubsubhelper.golang.org, Go LUCI, Michael Matloob, Michael Matloob, golang-co...@googlegroups.com
Attention needed from Ian Alexander
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
Attention is currently required from:
- Ian Alexander
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
4:49 PM (2 hours ago) 4:49 PM
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Michael Matloob, Michael Matloob, golang-co...@googlegroups.com
Ian Alexander submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/cmd/go/internal/doc/doc.go
Insertions: 1, Deletions: 1.
@@ -378,7 +378,7 @@
if isDotSlash(arg) {
arg = filepath.Join(wd, arg)
}
- if version != "" && (isDotSlash(arg) || filepath.IsAbs(arg)) {
+ if version != "" && (build.IsLocalImport(filepath.ToSlash(arg)) || filepath.IsAbs(arg)) {
log.Fatal("cannot use @version with local or absolute paths")
}
```
```
The name of the file: src/cmd/go/internal/doc/doc_test.go
Insertions: 11, Deletions: 12.
@@ -6,7 +6,6 @@
import (
"bytes"
- "context"
"flag"
"go/build"
"internal/testenv"
@@ -911,7 +910,7 @@
var flagSet flag.FlagSet
var logbuf bytes.Buffer
log.SetOutput(&logbuf)
- err := do(context.Background(), &b, &flagSet, test.args)
+ err := do(t.Context(), &b, &flagSet, test.args)
if err != nil {
t.Fatalf("%s %v: %s\n", test.name, test.args, err)
}
@@ -964,7 +963,7 @@
// Make sure crypto/rand does not have the symbol.
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"crypto/rand.float64"})
+ err := do(t.Context(), &b, &flagSet, []string{"crypto/rand.float64"})
if err == nil {
t.Errorf("expected error from crypto/rand.float64")
} else if !strings.Contains(err.Error(), "no symbol float64") {
@@ -974,7 +973,7 @@
// Make sure math/rand does have the symbol.
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"math/rand.float64"})
+ err := do(t.Context(), &b, &flagSet, []string{"math/rand.float64"})
if err != nil {
t.Errorf("unexpected error %q from math/rand.float64", err)
}
@@ -982,7 +981,7 @@
// Try the shorthand.
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"rand.float64"})
+ err := do(t.Context(), &b, &flagSet, []string{"rand.float64"})
if err != nil {
t.Errorf("unexpected error %q from rand.float64", err)
}
@@ -990,7 +989,7 @@
// Now try a missing symbol. We should see both packages in the error.
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"rand.doesnotexit"})
+ err := do(t.Context(), &b, &flagSet, []string{"rand.doesnotexit"})
if err == nil {
t.Errorf("expected error from rand.doesnotexit")
} else {
@@ -1028,21 +1027,21 @@
var b bytes.Buffer // We don't care about the output.
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"binary", "BigEndian"})
+ err := do(t.Context(), &b, &flagSet, []string{"binary", "BigEndian"})
if err != nil {
t.Errorf("unexpected error %q from binary BigEndian", err)
}
}
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"rand", "Float64"})
+ err := do(t.Context(), &b, &flagSet, []string{"rand", "Float64"})
if err != nil {
t.Errorf("unexpected error %q from rand Float64", err)
}
}
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"bytes", "Foo"})
+ err := do(t.Context(), &b, &flagSet, []string{"bytes", "Foo"})
if err == nil {
t.Errorf("expected error from bytes Foo")
} else if !strings.Contains(err.Error(), "no symbol Foo") {
@@ -1051,7 +1050,7 @@
}
{
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"nosuchpackage", "Foo"})
+ err := do(t.Context(), &b, &flagSet, []string{"nosuchpackage", "Foo"})
if err == nil {
// actually present in the user's filesystem
} else if !strings.Contains(err.Error(), "no such package") {
@@ -1072,7 +1071,7 @@
var b strings.Builder
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"./template"})
+ err := do(t.Context(), &b, &flagSet, []string{"./template"})
if err != nil {
t.Errorf("unexpected error %q from ./template", err)
}
@@ -1090,7 +1089,7 @@
maybeSkip(t)
var b strings.Builder
var flagSet flag.FlagSet
- err := do(context.Background(), &b, &flagSet, []string{"template.ZZZ"})
+ err := do(t.Context(), &b, &flagSet, []string{"template.ZZZ"})
// Expect an error.
if err == nil {
t.Error("expect an error for template.zzz")
```
```
The name of the file: src/cmd/go/testdata/script/issue_63696.txt
Insertions: 0, Deletions: 2.
@@ -1,7 +1,5 @@
go mod init m
go get rsc.io/quote@latest
-go mod tidy
-go build
go doc rsc.io/quote
cmp stdout want-latest.txt
go doc rsc.io/quote@latest
```
```
The name of the file: src/cmd/go/internal/doc/mod.go
Insertions: 10, Deletions: 17.
@@ -7,11 +7,11 @@
import (
"context"
"debug/buildinfo"
+ "fmt"
"go/build"
"os/exec"
- "path/filepath"
- "strings"
+ "cmd/go/internal/load"
"cmd/go/internal/modload"
)
@@ -22,25 +22,18 @@
loaderState.RootMode = modload.NoRoot
modload.Init(loaderState)
- noneSelected := func(path string) (version string) { return "none" }
- qrs, err := modload.QueryPackages(loaderState, ctx, pkgPath, version, noneSelected, nil)
+ var opts load.PackageOpts
+ args := []string{
+ fmt.Sprintf("%s@%s", pkgPath, version),
+ }
+ pkgs, err := load.PackagesAndErrorsOutsideModule(loaderState, ctx, opts, args)
if err != nil {
return nil, err
}
- qr := qrs[0]
-
- dir, err := loaderState.Fetcher().Download(ctx, qr.Mod)
- if err != nil {
- return nil, err
+ if len(pkgs) != 1 {
+ return nil, fmt.Errorf("incorrect number of packages: want 1, got %d", len(pkgs))
}
-
- // Calculate the directory for the package within the module.
- if len(qr.Packages) > 0 && qr.Packages[0] != qr.Mod.Path {
- rel := strings.TrimPrefix(qr.Packages[0], qr.Mod.Path+"/")
- dir = filepath.Join(dir, filepath.FromSlash(rel))
- }
-
- return buildCtx.ImportDir(dir, build.ImportComment)
+ return pkgs[0].Internal.Build, nil
}
// inferVersion checks if the argument matches a command on $PATH and returns its module path and version.
```
Change information
Commit message:
cmd/go/internal/doc: support @version suffix on first argument
This change allows `go doc` to display documentation for packages
outside the workspace by explicitly providing a version (e.g., `go doc
pkg@version`) or by inferring it from an installed command.
Therefore, all of the following are now valid:
```
go doc tx...@v0.13.0
go doc tx...@v0.13.0 FS
go doc txta...@v0.13.0
go doc golang.org/x/tools/tx...@v0.13.0
go doc golang.org/x/tools/tx...@v0.13.0 FS
go doc golang.org/x/tools/txta...@v0.13.0
```
Fixes #63696.
Change-Id: I22fb68e29c7f62bbe0bb240b82e5b789edc653f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/747380
Reviewed-by: Michael Matloob <mat...@golang.org>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <mat...@google.com>
Files:
- M src/cmd/go/internal/doc/doc.go
- M src/cmd/go/internal/doc/doc_test.go
- A src/cmd/go/internal/doc/mod.go
- A src/cmd/go/testdata/script/issue_63696.txt
Change size: L
Delta: 4 files changed, 237 insertions(+), 34 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by Michael Matloob, +2 by Michael Matloob
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages