Skip to content

Glossary page is corrupted #1041

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants