Skip to content

godoc link#24

Open
ccoVeille wants to merge 2 commits into
chocolacula:mainfrom
ccoveille-forks:godoc-link
Open

godoc link#24
ccoVeille wants to merge 2 commits into
chocolacula:mainfrom
ccoveille-forks:godoc-link

Conversation

@ccoVeille
Copy link
Copy Markdown
Contributor

  • feat: use comment.Parse to parse document as Markdown
  • chore: refactor code to use an helper

Fixes #17

@ccoVeille
Copy link
Copy Markdown
Contributor Author

Did you have a chance to review?

@chocolacula
Copy link
Copy Markdown
Owner

I’ll try to review this weekend

@chocolacula
Copy link
Copy Markdown
Owner

Could you please give me an example how it affect the documentation. With and without this change.

@ccoVeille
Copy link
Copy Markdown
Contributor Author

ccoVeille commented Dec 15, 2024

Sure

  • Take any godoc
  • add a godoc link, so something like [fmt.Stringer]
  • generate documentation

You can think about more complex link, even in the current package.

For example, in your package you can add something like called by [NewPackage]

More information here

https://golang.org/doc/comment#doclinks

@ccoVeille
Copy link
Copy Markdown
Contributor Author

About the rendering

Take a look here

https://cs.opensource.google/go/go/+/refs/tags/go1.23.4:src/encoding/json/encode.go;l=36

You will see the godoc links in the godoc of Marshal

And here you can see the rendering
https://pkg.go.dev/encoding/json#Marshal

Screenshot_20241215_140537_Firefox.jpg

Everything I underlined in yellow are the rendered godoc links

@chocolacula
Copy link
Copy Markdown
Owner

I see the problem, agree, this feature is needed but not sure in the implementation. Need some time to rethink.

@ccoVeille
Copy link
Copy Markdown
Contributor Author

Gentle nudge @chocolacula

@chocolacula
Copy link
Copy Markdown
Owner

Sorry for the long absence, it's been a while since I checked this, I might be wrong somewhere but in general there are 2 problems:

  • PR, I'm sure, is not ready to merge because it doesn't cover all cases e.g. standard lib, non standard lib etc.
  • Cover all the cases for this time, I guess, impossible because standard go parser resolves links wrong and unpredictable

The last point is easy to check with basic [json.Marshaller.MarshalJSON] interface method link. In a doc comment it resolves depending on import in the file. During parsing the doc comment we could expect just a part of information to build http link. The MarshalJSON method looks great in comments but doesn't reflect to a link, the best you can expect is https://pkg.go.dev/encoding/json#Marshaler and I, to be honest, don't have an idea how to convert one to another and take everything including web sites prefix like pkg.go.dev or another, import aliases, and so on into account.

So if we cannot make it working 100% it's better to do nothing and keep it as text.

@ccoVeille
Copy link
Copy Markdown
Contributor Author

Thanks for your feedbacks.

While I get your points, I'm unsure it works the way you mention.

I will have to double check, but from what I remember.

The idea of the PR is not to add links wherever possible.

The idea is to render links in the same way pkgsite would render them locally, so the way pkg.go.dev would render them.

So if someone use json.Marshaler and json package is not imported or json refers to something else than the std one. It won't work on pkg.go.dev or with pkgsite locally.

Then I don't expect magic to happen on your tool then.

It's worth checking again.

I will on my side for now. But for now, while I appreciate the feedbacks, it doesn't seem related to the code I provided, but about the possible traps that exists when working with go doc links.

@chocolacula
Copy link
Copy Markdown
Owner

I should admit I didn't explain it well, but let me point to the same example as you mentioned above. It doesn't work 100% even in pkg.go.dev, in the sentence:

If no [Marshaler.MarshalJSON] method is present but the value implements [encoding.TextMarshaler](https://pkg.go.dev/encoding#TextMarshaler) instead, Marshal calls [encoding.TextMarshaler.MarshalText](https://pkg.go.dev/encoding#TextMarshaler.MarshalText) and encodes the result as a JSON string.

[MarshalJSON] wasn't rendered as a link, but it was in my vscode:

image

And it's not clear for me how to handle private repos and different doc sources like gdmd itself which is not a problem for links in the source code, so I guess the best option here is do nothing 😄

@ccoVeille
Copy link
Copy Markdown
Contributor Author

Thanks for your explanation, it's clearer.

I agree gopls (that your IDE is using) might have a feature to make it works.

While the code used by pkg.go.dev, and so pkgsite, doesn't.

I think the lib I used for the implementation in this PR, is the exact one used by pkgsite.

I'll look for an open issue about it.

I'll have to double check, but I don't feel like it's an issue in godmd with the current implementation I made.

I agree private repository might be a real issue to tackle separely, or simply never.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stdlib godoc links supports

2 participants