shiki icon indicating copy to clipboard operation
shiki copied to clipboard

[Bug] `Notation` transfomers remove empty annotated lines

Open lachieh opened this issue 1 year ago • 10 comments

Validations

  • [X] Follow our Code of Conduct
  • [X] Read the Contributing Guide.
  • [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.

Describe the bug

It seems that all of the notation transformers like transformerNotationHighlight and transformerNotationDiff that if the line is empty, the span.line is removed entirely.

For example, the following code run through codeToHtml:

hello.world() // [!code highlight]
// [!code highlight]

Results in:

<pre>
  <code>
    <span class="line highlighted"><span>hello</span><span>.</span><span>world</span><span>()</span></span>

  </code>
</pre>

But I would assume it should be:

<pre>
  <code>
    <span class="line highlighted"><span>hello</span><span>.</span><span>world</span><span>()</span></span>
    <span class="line highlighted"></span>
  </code>
</pre>

Reproduction

https://stackblitz.com/edit/vitejs-vite-axpred?file=main.js

Contributes

  • [x] I am willing to submit a PR to fix this issue
  • [x] I am willing to submit a PR with failing tests

lachieh avatar Feb 15 '24 19:02 lachieh

This is intentional and expected behavior. I am ok with exposing an option to toggle that, pr welcome

antfu avatar Feb 16 '24 09:02 antfu

Thanks for the reply. It feels a little unexpected since the span.line is not removed unless there is a notation comment. Updated example above to demonstrate. Is this documented?

How would you indicate a diff that includes a new line as part of the code change?

lachieh avatar Feb 16 '24 14:02 lachieh

I guess it's not well-documented. It was migrated from https://github.com/innocenzi/shiki-processor and I kept the original behavior.

You can do // [!code highlight:2] on the first line to highlight the following two lines.

antfu avatar Feb 16 '24 15:02 antfu

Thanks, that gets me across the line for now.

I still think that this is probably still a bug since it is doing the opposite of what the notation comment is suggesting it will. Combined with the transformerRemoveLineBreak transformer and a .line { display: block } then the output wraps onto the previous line.

I'd like to suggest that an option to toggle is added, but the default behavior be that it just does what the comment says it will. I'm happy to submit a PR.

lachieh avatar Feb 16 '24 16:02 lachieh

Small follow-up to document behavior. The multi-line notation // [!code highlight:2] also removes the starting span.line if the comment is on an otherwise empty line.

lachieh avatar Feb 20 '24 17:02 lachieh

Hey, I am writing this code block with Shiki: Screenshot 2024-04-14 at 12 45 46 PM

and the output is as follows: Screenshot 2024-04-14 at 12 45 54 PM

This is my shikiConfig:

    shikiConfig: {
      theme: 'github-dark',
      transformers: [transformerNotationDiff()],
    },

but I do not want the span.line if there's nothing in it. How do I get that?

rishi-raj-jain avatar Apr 14 '24 07:04 rishi-raj-jain

Hey @rishi-raj-jain, that is not the same issue as this one and it is probably not shiki that is responsible for that extra line. It's probably whatever you're using for markdown processing.

I have tested it in an example and the output looks like this:

<code>
  <span class="line">
    <span style="color:#B8A965">cd</span>
    <span style="color:#C98A7D"> my-app</span>
  </span>
  <span class="line">
    <span style="color:#80A665">npm</span>
    <span style="color:#C98A7D"> run dev</span>
  </span>
</code>

As a fix, you could add this to your css which will hide the span if it is the last child and it has nothing in it:

span.line:last-child:empty {
   display: none;
}

If you still believe it is a problem with shiki, I'd suggest creating a minimal reproducible example of your own and posting a new issue.

lachieh avatar Apr 14 '24 13:04 lachieh

Hey @lachieh,

Thank you for your quick informative response! I was searching for the empty lines issue and this is the closest thread I could find. Pardon the inconvenience caused 🙏

rishi-raj-jain avatar Apr 14 '24 15:04 rishi-raj-jain