Skip to content

Glossary page is corrupted #1041

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

Closed
ehuss opened this issue Jan 28, 2021 · 10 comments
Closed

Glossary page is corrupted #1041

ehuss opened this issue Jan 28, 2021 · 10 comments
Assignees

Comments

@ehuss
Copy link
Contributor

ehuss commented Jan 28, 2021

The glossary page is not rendering correctly. Currently, it looks like this:

image

It looks like this is caused by mdbook-toc. Looking at the implementation, it looks like it is using pulldown-cmark-to-cmark, which in my experience is not very reliable.

(EDIT: no aspersions meant for pulldown-cmark-to-cmark, it is a very hard problem to convert the events back to markdown.)

@igaray igaray self-assigned this Jan 28, 2021
@igaray
Copy link
Member

igaray commented Jan 29, 2021

I think I found the problem.

The <span> element in the tables is triggering pulldown-cmark-to-cmark to include a spurious newline, which causes the following table cell to be re-rendered in markdown as a new row.

This change was introduced in pulldown-cmark-to-cmark here

Still thinking what we can do about it.

@camelid
Copy link
Member

camelid commented Jan 29, 2021

This doesn't solve the underlying problem, but we could change the layout of the Glossary to be like:

#### arena/arena allocation

... info about arenas

#### TyCtxt

... info about TyCtxt

which should "solve" the issue. I think the current Glossary layout is hard to read and navigate as it is, this approach would mean:

  • We can get of the span tags, which are kind of a hacky approach
  • We can more easily link directly to glossary items
  • The glossary would be 100% pure Markdown, no HTML
  • It would be easier to add items to the glossary
  • It would be easier to read the glossary

So, why not? ;)

@igaray
Copy link
Member

igaray commented Jan 29, 2021

I like this approach, if there's consensus I'll report the root cause upstream and "fix" ours this way.

@ehuss
Copy link
Contributor Author

ehuss commented Jan 29, 2021

That is how it is done with Cargo and the reference. It takes up a lot more space, but overall I think it works.

@VitalyAnkh
Copy link

Could I make a PR to solve this in @ehuss 's approach?

@igaray
Copy link
Member

igaray commented Feb 13, 2021

@VitalyAnkh absolutely! I had started on this but hadn't gotten to fixing all the references in the rest of the book. Go ahead!

@VitalyAnkh
Copy link

I find it's hard to add link without <span> element..

@LeSeulArtichaut
Copy link
Contributor

I think mdbook creates an ID for each title element?

@VitalyAnkh
Copy link

@LeSeulArtichaut Thanks! <span>s removed!

@rylev
Copy link
Member

rylev commented Jul 4, 2021

This has been fixed by #1146.

@rylev rylev closed this as completed Jul 4, 2021
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 a pull request may close this issue.

6 participants