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

Add source map for every block-level element in the Markdown preview #133376

Closed
Lemmingh opened this issue Sep 18, 2021 · 4 comments · Fixed by #134799
Closed

Add source map for every block-level element in the Markdown preview #133376

Lemmingh opened this issue Sep 18, 2021 · 4 comments · Fixed by #134799
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities insiders-released Patch has been released in VS Code Insiders markdown Markdown support issues verified Verification succeeded
Milestone

Comments

@Lemmingh
Copy link
Contributor

Issue Type: Bug

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

  • VS Code Version: 1.61.0-insider (system setup) (a0af581)
  • OS Version: Windows_NT x64 10.0.19043

I searched the Issues by https://github.com/microsoft/vscode/issues?q=is%3Aissue+label%3Amarkdown+scroll, found some cases with similar behavior, but couldn't recognize one as the same. So, I open this issue in case a root cause of bugs has't been revealed.

Steps to Reproduce

  1. Create a Markdown editor, and fill it with one of the samples below.

  2. Adjust the window size, so that no more than 30 lines are visible in the editor.

  3. "Open Preview to the Side".

  4. Scroll the preview.

Actual behavior:

The editor is scrolled strangely, or jumps to an unexpected position, or whatever.

Expected behavior:

The editor and the preview should be synchronized, all or most of the time.

Problem

The Markdown preview only sets source map for a few kinds of elements, which can lead to severely inaccurate scrolling:

for (const renderName of ['paragraph_open', 'heading_open', 'image', 'code_block', 'fence', 'blockquote_open', 'list_item_open']) {
this.addLineNumberRenderer(md, renderName);
}

It can be observed in many conditions, such as:

  • Scrolling the preview in order to find something in the editor.

  • Editing a long document when the preview is open.

Solution

Add source map for every element where possible.

Here is a solution. I've tested it by adding a Markdown extension.

First, internally create a markdown-it plugin in the markdownEngine.ts:

import type { PluginSimple } from "markdown-it";

/**
 * Adds begin line index to the output via the "data-line" data attribute.
 */
const pluginSourceMap: PluginSimple = (md): void => {
    // Set the attribute on every possible token.
    md.core.ruler.push("data_attribute_source_map", (state): any => {
        for (const token of state.tokens) {
            if (token.map && token.type !== "inline") {
                token.attrSet("data-line", String(token.map[0]));
                token.attrJoin("class", "code-line");
            }
        }
    });

    // The "html_block" renderer doesn't respect `attrs`. We need to insert a marker.
    const originalHtmlBlockRenderer = md.renderer.rules["html_block"];
    if (originalHtmlBlockRenderer) {
        md.renderer.rules["html_block"] = (tokens, idx, options, env, self) => {
            return (
                `<div ${self.renderAttrs(tokens[idx])} ></div>\n` +
                originalHtmlBlockRenderer(tokens, idx, options, env, self)
            );
        };
    }
};

Then, load it just before the return:

md.use(pluginSourceMap);

Known limitations:

Some markdown-it plugins do not respect attrs, thus, their output won't get source map.

Sample 1

From:

Repeat this piece 10 times:

<section>
a<br>
a
a
a
a
a
a
a
</section>

Sample 2

From:

Begin the document with:

#

Then, add 99 blank lines.

Next, add:

|a|b|
| --- | --- |

Finally, repeat this line 99 times:

|c|d|
@mjbvz
Copy link
Collaborator

mjbvz commented Sep 27, 2021

Thanks or the investigation! Can you please submit a PR for this

@mjbvz mjbvz added markdown Markdown support issues help wanted Issues identified as good community contribution opportunities labels Sep 27, 2021
@Lemmingh
Copy link
Contributor Author

Lemmingh commented Oct 5, 2021

I'd love to, but I'm still waiting for DefinitelyTyped to fix type definitions.

@mjbvz mjbvz added the bug Issue identified by VS Code Team member as probable bug label Oct 11, 2021
@mjbvz mjbvz added this to the October 2021 milestone Oct 12, 2021
@mjbvz mjbvz added the author-verification-requested Issues potentially verifiable by issue author label Oct 12, 2021
@Lemmingh
Copy link
Contributor Author

/verified

<section>
a<br>
a
</section>

|a|b|
| --- | --- |
|c|d|

now gives

<div data-line="0" class="code-line"></div>
<section>
a<br>
a
</section>
<table data-line="5" class="code-line">
<thead data-line="5" class="code-line">
<tr data-line="5" class="code-line">
<th>a</th>
<th>b</th>
</tr>
</thead>
<tbody data-line="7" class="code-line">
<tr data-line="7" class="code-line">
<td>c</td>
<td>d</td>
</tr>
</tbody>
</table>

<div class="code-line" data-line="8"></div>

And the scrolling for Sample 2 is also more accurate.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities insiders-released Patch has been released in VS Code Insiders markdown Markdown support issues verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@mjbvz @Lemmingh and others