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

x/pkgsite: consider moving to goldmark to handle issues in READMEs #39297

Closed
hhrutter opened this issue May 28, 2020 · 6 comments
Closed

x/pkgsite: consider moving to goldmark to handle issues in READMEs #39297

hhrutter opened this issue May 28, 2020 · 6 comments

Comments

@hhrutter
Copy link

@hhrutter hhrutter commented May 28, 2020

What is the URL of the page with the issue?

https://pkg.go.dev/mod/github.com/pdfcpu/pdfcpu

What is your user agent?

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.138 Safari/537.36

Some images don't show up and there is also a markdown table that's screwed up.

@gopherbot gopherbot added the pkgsite label May 28, 2020
@gopherbot gopherbot added this to the Unreleased milestone May 28, 2020
@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented May 28, 2020

Thanks for the feedback @hhrutter - we'll work on a fix! This is also related to #37194

@julieqiu julieqiu added the NeedsFix label May 28, 2020
@julieqiu julieqiu changed the title go.dev: readme.md not rendered properly go.dev: broken image links and markdown table in README for github.com/pdfcpu/pdfcpu May 28, 2020
@julieqiu julieqiu changed the title go.dev: broken image links and markdown table in README for github.com/pdfcpu/pdfcpu x/pkgsite: broken image links and markdown table in README for github.com/pdfcpu/pdfcpu Jun 15, 2020
@josh-newman
Copy link

@josh-newman josh-newman commented Aug 5, 2020

In case it's helpful, I ran into another case of incorrect table rendering, this time with no images in the table: correct on Github (snapshot), incorrect on pkg.go.dev (snapshot).

@shaqque
Copy link
Contributor

@shaqque shaqque commented Aug 5, 2020

The broken markdown tables for both packages (pdfcpu and parquet-go) seem to be caused by incorrectly formatted markdown, at least according to blackfriday (the external markdown parser pkg.go.dev uses). Note that Github-flavored-markdown (GFM), which Github uses, is not the same as regular markdown.

Specifically,

  1. pdfcpu's table does not does not have a space/newline before the start of the table, causing blackfriday to not detect it as a table. Adding that space allows blackfriday to detect and render the table correctly.
  2. parquet-go's table uses 1 hyphen per column in the header row instead of 3, which blackfriday requires (see russross/blackfriday#351). This also appears to violate Github's documentation for tables, though the official Github-flavored-markdown spec doesn't say anything about this despite the examples all having 3 hyphens per column. Changing the table to use 3 hyphens per column in the header row allows blackfriday to detect and render the table correctly.

There is a related discussion on #40203 about blackfriday and GFM. In particular, @zikaeroh suggested

Unless I'm mistaken, blackfriday isn't actively developed anymore, and many have moved onto goldmark. Hugo, gitea, and even x/tools/present and x/website use goldmark for their markdown support.

Perhaps it'd be better to move to a markdown library which is actively maintained.

I tried the above cases using goldmark, and in both cases, the table was identified and rendered properly by goldmark.

I think this, along with active maintenance (last commit was 6 days ago) and broader use among other Go projects, makes moving to goldmark a good idea.

@julieqiu julieqiu changed the title x/pkgsite: broken image links and markdown table in README for github.com/pdfcpu/pdfcpu x/pkgsite: consider moving to goldmark to issues in READMEs Aug 13, 2020
@julieqiu julieqiu changed the title x/pkgsite: consider moving to goldmark to issues in READMEs x/pkgsite: consider moving to goldmark to handle issues in READMEs Aug 19, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 2, 2020

Change https://golang.org/cl/266579 mentions this issue: content/static: copy readme.css into legacy_readme.css

gopherbot pushed a commit to golang/pkgsite that referenced this issue Nov 2, 2020
Code in motion. Preparing to generate new readme styles for
the goldmark parser.

For golang/go#39297

Change-Id: I506046ba36fdef6fa935b0600e1037e2d832ad24
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/266579
Trust: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 3, 2020

Change https://golang.org/cl/267117 mentions this issue: internal/frontend: separate goldmark readme code from legacy overview code

gopherbot pushed a commit to golang/pkgsite that referenced this issue Nov 3, 2020
… code

The overview file contains mostly legacy code used
to construct the overview page. This change separates
the goldmark code in preparation for updates that will
generate a TOC for the readme and the future removal of
all overview related code.

For golang/go#39297

Change-Id: Ifa5d0ee3983478fd25c6c59fc1bd2c45457cc05c
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/267117
Run-TryBot: Jamal Carvalho <jamal@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Trust: Jamal Carvalho <jamal@golang.org>
@jamalc jamalc self-assigned this Nov 18, 2020
@jamalc
Copy link
Member

@jamalc jamalc commented Nov 18, 2020

@hhrutter - this should be fixed. We made the switch to goldmark 😊

@jamalc jamalc closed this Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants