internal/frontend: show module readme links in sub-units
Show a module's README links on all units within the module.
For golang/go#42968
Change-Id: I8701a04a16e1dd766b1b23ce12d52d01190840fd
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/275276
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/content/static/html/helpers/_unit_meta.tmpl b/content/static/html/helpers/_unit_meta.tmpl
index 21ed428..3ddc1b9 100644
--- a/content/static/html/helpers/_unit_meta.tmpl
+++ b/content/static/html/helpers/_unit_meta.tmpl
@@ -15,13 +15,15 @@
{{else}}
Repository URL not available.
{{end}}
- {{range .Details.ReadmeLinks}}
- <div class="UnitMeta-repo">{{.Body}}</div>
- <a href="{{.Href}}" title="{{.Href}}" target="_blank" rel="noopener">{{.Href}}</a>
- {{end}}
- {{range .Details.DocLinks}}
- <div class="UnitMeta-repo">{{.Body}}</div>
- <a href="{{.Href}}" title="{{.Href}}" target="_blank" rel="noopener">{{.Href}}</a>
- {{end}}
+ {{template "unit_meta_links" .Details.ReadmeLinks}}
+ {{template "unit_meta_links" .Details.DocLinks}}
+ {{template "unit_meta_links" .Details.ModuleReadmeLinks}}
</div>
{{end}}
+
+{{define "unit_meta_links"}}
+ {{range .}}
+ <div class="UnitMeta-repo">{{.Body}}</div>
+ <a href="{{.Href}}" title="{{.Href}}" target="_blank" rel="noopener">{{.Href}}</a>
+ {{end}}
+{{end}}
\ No newline at end of file
diff --git a/internal/datasource.go b/internal/datasource.go
index 0747af4..6f886c1 100644
--- a/internal/datasource.go
+++ b/internal/datasource.go
@@ -21,4 +21,6 @@
GetUnit(ctx context.Context, pathInfo *UnitMeta, fields FieldSet) (_ *Unit, err error)
// GetUnitMeta returns information about a path.
GetUnitMeta(ctx context.Context, path, requestedModulePath, requestedVersion string) (_ *UnitMeta, err error)
+ // GetModuleReadme gets the readme for the module.
+ GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (*Readme, error)
}
diff --git a/internal/frontend/readme.go b/internal/frontend/readme.go
index 09b1de8..b2871e0 100644
--- a/internal/frontend/readme.go
+++ b/internal/frontend/readme.go
@@ -24,6 +24,7 @@
"github.com/yuin/goldmark/util"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
+ "golang.org/x/pkgsite/internal/source"
)
// Heading holds data about a heading within a readme used in the
@@ -61,12 +62,16 @@
// This function is exported for use by external tools.
func ProcessReadme(ctx context.Context, u *internal.Unit) (_ *Readme, err error) {
defer derrors.Wrap(&err, "ProcessReadme(%q, %q, %q)", u.Path, u.ModulePath, u.Version)
- if u.Readme == nil || u.Readme.Contents == "" {
+ return processReadme(u.Readme, u.SourceInfo)
+}
+
+func processReadme(readme *internal.Readme, sourceInfo *source.Info) (_ *Readme, err error) {
+ if readme == nil || readme.Contents == "" {
return &Readme{}, nil
}
- if !isMarkdown(u.Readme.Filepath) {
+ if !isMarkdown(readme.Filepath) {
t := template.Must(template.New("").Parse(`<pre class="readme">{{.}}</pre>`))
- h, err := t.ExecuteToHTML(u.Readme.Contents)
+ h, err := t.ExecuteToHTML(readme.Contents)
if err != nil {
return nil, err
}
@@ -93,8 +98,8 @@
// before it is rendered.
parser.WithASTTransformers(
util.Prioritized(&astTransformer{
- info: u.SourceInfo,
- readme: u.Readme,
+ info: sourceInfo,
+ readme: readme,
}, astTransformerPriority),
// Extract links after we have transformed the URLs.
util.Prioritized(el, astTransformerPriority+1),
@@ -110,10 +115,10 @@
)
gdMarkdown.Renderer().AddOptions(
renderer.WithNodeRenderers(
- util.Prioritized(newHTMLRenderer(u.SourceInfo, u.Readme), 100),
+ util.Prioritized(newHTMLRenderer(sourceInfo, readme), 100),
),
)
- contents := []byte(u.Readme.Contents)
+ contents := []byte(readme.Contents)
gdParser := gdMarkdown.Parser()
reader := gmtext.NewReader(contents)
pctx := parser.NewContext(parser.WithIDs(newIDs()))
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index 6af263c..707d1e9 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -81,7 +81,7 @@
// Units with this prefix will be marked as excluded.
const excludedModulePath = "github.com/module/excluded"
-const linksReadme = `
+const moduleLinksReadme = `
# Heading
Stuff
@@ -90,6 +90,11 @@
- [title2](javascript://pwned)
`
+const packageLinksReadme = `
+# Links
+- [pkg title](http://url2)
+`
+
var testModules = []testModule{
{
// An ordinary module, with three versions.
@@ -160,15 +165,29 @@
},
{
// A module with links.
- path: "github.com/links",
+ path: "github.com/links/mod",
redistributable: true,
versions: []string{"v1.0.0"},
packages: []testPackage{
{
- suffix: "pkg",
+ suffix: "",
+ readmeFilePath: sample.ReadmeFilePath, // required
+ readmeContents: moduleLinksReadme,
+ },
+ {
+ // This package has no readme, just links in its godoc. So the
+ // UnitMeta (right sidebar) links will be from the godoc and
+ // module readme.
+ suffix: "no_readme",
+ doc: sample.DocumentationHTML.String(),
+ },
+ {
+ // This package has a readme as well as godoc links, so the
+ // UnitMeta links will be from all three places.
+ suffix: "has_readme",
+ readmeFilePath: "has_readme/README.md", // required
+ readmeContents: packageLinksReadme,
doc: sample.DocumentationHTML.String(),
- readmeContents: linksReadme,
- readmeFilePath: sample.ReadmeFilePath,
},
},
},
@@ -680,6 +699,54 @@
}
}
+func checkLink(n int, title, url, body string) htmlcheck.Checker {
+ // The first div under .UnitMeta is "Links" and the second is the
+ // repository, so div numbers below start with 3. There is no "a" for
+ // "Links", so the "a" numbers are off by one.
+ return in("",
+ in(fmt.Sprintf("div:nth-of-type(%d)", n+2), hasText(title)),
+ in(fmt.Sprintf("a:nth-of-type(%d)", n+1), href(url), hasText(body)))
+}
+
+var linksTestCases = []serverTestCase{
+ {
+ name: "module links",
+ urlPath: "/github.com/links/mod",
+ wantStatusCode: http.StatusOK,
+ want: in(".UnitMeta",
+ // Module readme links.
+ checkLink(1, "title1", "http://url1", "http://url1"),
+ checkLink(2, "title2", "about:invalid#zGoSafez", "javascript://pwned"),
+ ),
+ },
+ {
+ name: "no_readme package links",
+ urlPath: "/github.com/links/mod/no_readme",
+ wantStatusCode: http.StatusOK,
+ want: in(".UnitMeta",
+ // Package doc links are first.
+ checkLink(1, "pkg.go.dev", "https://pkg.go.dev", "https://pkg.go.dev"),
+ // Then module readmes.
+ checkLink(2, "title1", "http://url1", "http://url1"),
+ checkLink(3, "title2", "about:invalid#zGoSafez", "javascript://pwned"),
+ ),
+ },
+ {
+ name: "has_readme package links",
+ urlPath: "/github.com/links/mod/has_readme",
+ wantStatusCode: http.StatusOK,
+ want: in(".UnitMeta",
+ // Package readme links are first.
+ checkLink(1, "pkg title", "http://url2", "http://url2"),
+ // Package doc links are second.
+ checkLink(2, "pkg.go.dev", "https://pkg.go.dev", "https://pkg.go.dev"),
+ // Module readme links are third.
+ checkLink(3, "title1", "http://url1", "http://url1"),
+ checkLink(4, "title2", "about:invalid#zGoSafez", "javascript://pwned"),
+ ),
+ },
+}
+
// TestServer checks the contents of served pages by looking for
// strings and elements in the parsed HTML response body.
//
@@ -705,21 +772,7 @@
{
name: "goldmark and readme outline experiments",
testCasesFunc: func() []serverTestCase {
- return append(serverTestCases(), serverTestCase{
- name: "links",
- urlPath: "/github.com/links/pkg",
- wantStatusCode: http.StatusOK,
- // There is one div before the actual links appear, hence
- // the numbers are off by one.
- want: in(".UnitMeta",
- in("div:nth-of-type(3)", hasText("title1")),
- in("a:nth-of-type(2)", href("http://url1"), hasText("http://url1")),
- in("div:nth-of-type(4)", hasText("title2")),
- in("a:nth-of-type(3)", href("about:invalid#zGoSafez"), hasText("javascript://pwned")),
- in("div:nth-of-type(5)", hasText("pkg.go.dev")),
- in("a:nth-of-type(4)", href("https://pkg.go.dev"), hasText("https://pkg.go.dev")),
- ),
- })
+ return append(serverTestCases(), linksTestCases...)
},
experiments: []string{internal.ExperimentReadmeOutline, internal.ExperimentGoldmark},
},
diff --git a/internal/frontend/unit_main.go b/internal/frontend/unit_main.go
index 4ad91da..42468b4 100644
--- a/internal/frontend/unit_main.go
+++ b/internal/frontend/unit_main.go
@@ -51,14 +51,19 @@
// used to render the readme outline in the sidebar.
ReadmeOutline []*Heading
- // ReadmeLinks are from the "Links" section of the readme file,
- // and are displayed on the right sidebar.
+ // ReadmeLinks are from the "Links" section of this unit's readme file, and
+ // are displayed on the right sidebar.
ReadmeLinks []link
// DocLinks are from the "Links" section of the Go package documentation,
// and are displayed on the right sidebar.
DocLinks []link
+ // ModuleReadmeLinks are from the "Links" section of this unit's module, if
+ // the unit is not itself a module. They are displayed on the right sidebar.
+ // See https://golang.org/issue/42968.
+ ModuleReadmeLinks []link
+
// ImportedByCount is the number of packages that import this path.
// When the count is > limit it will read as 'limit+'. This field
// is not supported when using a datasource proxy.
@@ -127,9 +132,9 @@
return nil, err
}
var (
- docParts = &dochtml.Parts{}
- docLinks []link
- files []*File
+ docParts = &dochtml.Parts{}
+ docLinks, modLinks []link
+ files []*File
)
if unit.Documentation != nil {
end := middleware.ElapsedStat(ctx, "DecodePackage")
@@ -156,25 +161,42 @@
files = sourceFiles(unit, docPkg)
end()
}
+ // If the unit is not a module, fetch the module readme to extract its
+ // links.
+ // In the unlikely event that the module is redistributable but the unit is
+ // not, we will not show the module links on the unit page.
+ if unit.Path != unit.ModulePath && unit.IsRedistributable && experiment.IsActive(ctx, internal.ExperimentGoldmark) {
+ modReadme, err := ds.GetModuleReadme(ctx, unit.ModulePath, unit.Version)
+ if err != nil {
+ return nil, err
+ }
+ rm, err := processReadme(modReadme, um.SourceInfo)
+ if err != nil {
+ return nil, err
+ }
+ modLinks = rm.Links
+ }
+
return &MainDetails{
- ExpandReadme: expandReadme,
- NestedModules: nestedModules,
- Subdirectories: subdirectories,
- Licenses: transformLicenseMetadata(um.Licenses),
- CommitTime: absoluteTime(um.CommitTime),
- Readme: readme.HTML,
- ReadmeOutline: readme.Outline,
- ReadmeLinks: readme.Links,
- DocLinks: docLinks,
- DocOutline: docParts.Outline,
- DocBody: docParts.Body,
- SourceFiles: files,
- RepositoryURL: um.SourceInfo.RepoURL(),
- SourceURL: um.SourceInfo.DirectoryURL(internal.Suffix(um.Path, um.ModulePath)),
- MobileOutline: docParts.MobileOutline,
- NumImports: unit.NumImports,
- ImportedByCount: importedByCount,
- IsPackage: unit.IsPackage(),
+ ExpandReadme: expandReadme,
+ NestedModules: nestedModules,
+ Subdirectories: subdirectories,
+ Licenses: transformLicenseMetadata(um.Licenses),
+ CommitTime: absoluteTime(um.CommitTime),
+ Readme: readme.HTML,
+ ReadmeOutline: readme.Outline,
+ ReadmeLinks: readme.Links,
+ DocLinks: docLinks,
+ ModuleReadmeLinks: modLinks,
+ DocOutline: docParts.Outline,
+ DocBody: docParts.Body,
+ SourceFiles: files,
+ RepositoryURL: um.SourceInfo.RepoURL(),
+ SourceURL: um.SourceInfo.DirectoryURL(internal.Suffix(um.Path, um.ModulePath)),
+ MobileOutline: docParts.MobileOutline,
+ NumImports: unit.NumImports,
+ ImportedByCount: importedByCount,
+ IsPackage: unit.IsPackage(),
}, nil
}
diff --git a/internal/localdatasource/datasource.go b/internal/localdatasource/datasource.go
index 116b74b..94ee63e 100644
--- a/internal/localdatasource/datasource.go
+++ b/internal/localdatasource/datasource.go
@@ -181,3 +181,8 @@
func (ds *DataSource) GetNestedModules(ctx context.Context, modulePath string) ([]*internal.ModuleInfo, error) {
return nil, nil
}
+
+// GetModuleReadme is not implemented.
+func (*DataSource) GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (*internal.Readme, error) {
+ return nil, nil
+}
diff --git a/internal/postgres/unit.go b/internal/postgres/unit.go
index 49e9a15..8a2fec9 100644
--- a/internal/postgres/unit.go
+++ b/internal/postgres/unit.go
@@ -404,3 +404,28 @@
}
return paths, nil
}
+
+// GetModuleReadme returns the README corresponding to the modulePath and version.
+func (db *DB) GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (_ *internal.Readme, err error) {
+ defer derrors.Wrap(&err, "GetModuleReadme(ctx, %q, %q)", modulePath, resolvedVersion)
+ var readme internal.Readme
+ err = db.db.QueryRow(ctx, `
+ SELECT file_path, contents
+ FROM modules m
+ INNER JOIN units u
+ ON u.module_id = m.id
+ INNER JOIN readmes r
+ ON u.id = r.unit_id
+ WHERE
+ m.module_path=$1
+ AND m.version=$2
+ AND m.module_path=u.path`, modulePath, resolvedVersion).Scan(&readme.Filepath, &readme.Contents)
+ switch err {
+ case sql.ErrNoRows:
+ return nil, derrors.NotFound
+ case nil:
+ return &readme, nil
+ default:
+ return nil, err
+ }
+}
diff --git a/internal/proxydatasource/details.go b/internal/proxydatasource/details.go
index 37da084..6171df1 100644
--- a/internal/proxydatasource/details.go
+++ b/internal/proxydatasource/details.go
@@ -71,3 +71,8 @@
func (ds *DataSource) GetNestedModules(ctx context.Context, modulePath string) (_ []*internal.ModuleInfo, err error) {
return nil, nil
}
+
+// GetModuleReadme is unimplemented.
+func (ds *DataSource) GetModuleReadme(ctx context.Context, modulePath, resolvedVersion string) (*internal.Readme, error) {
+ return nil, nil
+}
