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

Jump to bottom

x/website: syntax rules in spec are not hyperlinked anymore #50915

Closed
griesemer opened this issue Jan 30, 2022 · 4 comments
Closed

x/website: syntax rules in spec are not hyperlinked anymore #50915

griesemer opened this issue Jan 30, 2022 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@griesemer
Copy link
Contributor

In the (distant?) past, the syntax rules (EBNF grammar) were hyperlinked: clicking on the name of a production lead to the corresponding definition. This was very helpful to explore the grammar. The spec also used some of the link targets in the prose, some of which are now stale (such as MethodName). The recent CL 381954 tries to fix these by changing the links to section headers, but that is not the intent of those links - they should point to the grammar productions.

The spec source text (doc/go_spec.html) doesn't contain the link information, it is automatically hyperlinked by running Linkify (website/internal/spec/spec.go) which appears to be called from serveHTML (website/internal/web/site.go). For some reason it is not run, though. As an aside, the hyperlinking also checks that the EBNF is syntactically correct, another benefit.

Looking at the source of the served spec I see that it starts with <!DOCTYPE. The code in serveHTML checks for this and exits early. Maybe it's just a matter of swapping the order of the if's in that code? (That said, I'm not familiar with the details of how the website is created anymore, and where the <!DOCTYPE is added (it's not in doc/go_spec.html).

I suspect this should be fairly easy to fix if one knows where to look. It would be good to fix this for 1.18 as we have introduced new grammar. Also, having the EBNF syntax check and being able to jump to grammar productions is generally useful.

Not a release blocker because this appears to have been broken for a while.
@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 30, 2022
@griesemer griesemer added this to the Go1.18 milestone Jan 30, 2022
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Jan 30, 2022
@griesemer griesemer changed the title website: syntax rules in spec are not hyperlinked anymore x/website: syntax rules in spec are not hyperlinked anymore Feb 2, 2022
@griesemer
Copy link
Contributor Author

cc: @dmitshur @jamalc

@ianlancetaylor ianlancetaylor mentioned this issue Feb 3, 2022
Closed
@griesemer
Copy link
Contributor Author

Ping. Can somebody familiar with the current workings of the website server comment on this? It would be a shame to have lost this ability, especially because the code is actually present, just not used.

@dmitshur
Copy link
Contributor

dmitshur commented Mar 2, 2022 •
edited

I don't have a chance to look into this issue closely myself, but wanted to comment on this part, in case it helps point you in the right direction:

The code in serveHTML checks for this and exits early. Maybe it's just a matter of swapping the order of the if's in that code? (That said, I'm not familiar with the details of how the website is created anymore, and where the <!DOCTYPE is added (it's not in doc/go_spec.html).

In general, the Go website (primarily https://go.dev, but also other domains such as https://golang.org, https://tour.golang.org, https://golang.google.cn) is all served by the cmd/golangorg command in x/website, and its README is up to date and provides a good starting point. If you run the command locally, it'll serve the entire website, and that's a good way to check e.g. what happens if that if is swapped.

Looking at the source of the served spec I see that it starts with <!DOCTYPE.

The served HTML will always have the "<!DOCTYPE" prefix: it might be added automatically by internal/web, or it might coming from a .html content file that explicitly includes that prefix, which effectively disables the template wrapping.

(it's not in doc/go_spec.html)

That's the right place to look. Indeed, the go_spec.html content file doesn't have the "<!DOCTYPE" prefix, and so it's probably not relevant in this issue.

@gopherbot
Copy link

Change https://go.dev/cl/388859 mentions this issue: internal/web: fix syntax rules hyperlinks

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 4, 2022
passionSeven added a commit to passionSeven/website that referenced this issue Oct 18, 2022
@passionSeven
internal/web: fix syntax rules hyperlinks
cc7c7da 
The filepath for the spec document changed and broke
the linkify step.

Fixes golang/go#50915

Change-Id: Ieafe9c665f4063fb9f9ddf74610925f123e2c249
Reviewed-on: https://go-review.googlesource.com/c/website/+/388859
Run-TryBot: Jamal Carvalho <jamal@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Trust: Jamal Carvalho <jamalcarvalho@google.com>
@golang golang locked and limited conversation to collaborators Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
3 participants
@dmitshur @griesemer @gopherbot