JohnStarich / go

This is a collection of my Go modules in one easy-to-import place
Apache License 2.0
54 stars 13 forks source link

gopages: fix rendering of doclinks for go 1.19 comments #38

Closed phsym closed 1 year ago

phsym commented 1 year ago

Hi ! Thanks for this awesome gopages tool. I love it already.

I followed your recent note about go 1.19 and improved formatting, however I realized that doclinks where not rendered at all so I took the chance to try to improve your tool.

To achieve that I did 2 things :

I tried it on my codebase (sorry I can't share the full result) but here's a quick example

Without the change: image

With the change image

With the link properly sending to http://<host>:<port>/pkg/<module>/<package>/#New

DocLinks to doc from other modules or stdlib redirect to pkg.go.dev

phsym commented 1 year ago

Hum ok, maybe I shouldn't have update all dependencies, some may not be go 1.16 compatible anymore, let me try to fix that

phsym commented 1 year ago

Looks like latest golang.org/x/tools requires a version golang.org/x/sys which itself requires unsafe.Slice from stdlib which has been added in go 1.17. Would you be OK to drop go 1.16 compatibility ?

phsym commented 1 year ago

I removed go 1.16 from the github actions ci and it passed

phsym commented 1 year ago

@JohnStarich here's a quick example where it fails before this PR. I'll look at how to make this a test case

// Package example is a dummy package showing how comments doc is rendered.
//
// Here are some random doc links:
//   - Std library: [os.File] [*os.File] [encoding/xml.Encoder]
//   - Std imported: [json.Decoder] [json.Encoder]
//   - Packages: [os] [json] [encoding/xml]
package example

import (
    "encoding/json"
)

// JSONFunc does random stuff with [json.Decoder] & [json.Encoder]
func JSONFunc() {
    json.Marshal("foobar") // Just to import json
}

Not all doc-links are rendered as links, and those which are links to localhost/ which eads to a 404

image

After PR, links are properly rendered, a linked to pkg.go.dev when they are external dependencies (internal links are linked properly too, will add them to my test case)

image
phsym commented 1 year ago

I added a test case in gopages/internal/generate/generate_test.go

EDIT: I had to skip this test case when running in go version prior to 1.19

JohnStarich commented 1 year ago

I've released v0.1.20 with your change 🎉