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

HTML header contents in markdown notebook cells not correctly populated in breadcrumbs #134853

Closed
joyceerhl opened this issue Oct 12, 2021 · 5 comments
Assignees
Labels
breadcrumbs feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities notebook-markdown outline Source outline view issues verification-needed Verification of issue is requested verified Verification succeeded

Comments

@joyceerhl
Copy link
Collaborator

Does this issue occur when all extensions are disabled?: Yes/No

  • VS Code Version: github.dev
  • OS Version: Windows 10

Steps to Reproduce:

  1. Visit https://github.dev/lostintangent/python-tuples
  2. Select any of the markdown cells
  3. 🐛 breadcrumb preview says 'empty cell', this is apparently because the cell headers are created with HTML; if I rewrite them to be Markdown the breadcrumbs are accurate
    image
@mjbvz
Copy link
Collaborator

mjbvz commented Oct 12, 2021

Here's the relevant code:

for (const token of marked.lexer(content, { gfm: true })) {

We currently only look for markdown headers. Instead, we'd likely want to get the headers from the rendered markup. That would nicely support other types of rendered markup or extensions to the markdown rendering

@mjbvz mjbvz added the feature-request Request for new features or functionality label Oct 12, 2021
@mjbvz mjbvz added this to the Backlog milestone Oct 12, 2021
@IanMatthewHuff
Copy link
Member

This would also be a big improvement for Outline view I believe. Jupyter picks up HTML headers and adds them to the table of contents view, but in VS Code we miss out on them so the structure looks funny with some notebooks.

@rebornix rebornix removed the notebook label Oct 21, 2021
@rebornix rebornix removed their assignment Oct 21, 2021
@mjbvz mjbvz added the help wanted Issues identified as good community contribution opportunities label Nov 16, 2021
@roblourens roblourens modified the milestones: Backlog, August 2022 Jul 21, 2022
@roblourens roblourens assigned roblourens and unassigned mjbvz Jul 21, 2022
@rebornix rebornix assigned rebornix and unassigned roblourens Sep 6, 2022
@rebornix
Copy link
Member

rebornix commented Sep 7, 2022

Pushed a fix to parse the content for h1 to h6 headers written as HTML tags. It's synchronous and performant, doesn't require the full rendering of all markdown cells either. The catch with this is we are only finding headers, the markdown cells with <p> will still be rendered as empty cells in TOC.

cc @mjbvz what's your recommendation of improving this further more? marked plaintext renderer will skip all html tags, does it mean to get proper gist of a markdown cell, we do need to have it rendered in the webview?

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 7, 2022

@rebornix Do you mean handling cases where the user has written raw html tags inside their markdown? For that we could turn on supportHtml:

if (!markdown.supportHtml) {

That will let the tags through, which I think is ok so long as are only looking at the tokens and not actually rendering the resulting html

@rebornix rebornix added the verification-needed Verification of issue is requested label Sep 27, 2022
@mjbvz mjbvz added the verified Verification succeeded label Sep 27, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breadcrumbs feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities notebook-markdown outline Source outline view issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants
@roblourens @IanMatthewHuff @rebornix @mjbvz @joyceerhl and others