shikijs / shiki

A beautiful yet powerful syntax highlighter
http://shiki.style/
MIT License
9.68k stars 354 forks source link

[Bug] `Notation` transfomers remove empty annotated lines #589

Open lachieh opened 6 months ago

lachieh commented 6 months ago

Validations

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

antfu commented 6 months ago

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

lachieh commented 6 months ago

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?

antfu commented 6 months ago

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.

lachieh commented 6 months ago

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 commented 6 months ago

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.

rishi-raj-jain commented 4 months ago

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?

lachieh commented 4 months ago

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.

rishi-raj-jain commented 4 months ago

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 🙏

IgnaceMaes commented 2 months ago

We ran into this bug as well when migrating the Ember.js documentation website from Prism to Shiki. This feels like a very unexpected default.

As a workaround, a placeholder was put on empty lines with diffs before passing the code to codeToHtml. And afterwards, removing this placeholder again. (https://github.com/IgnaceMaes/ember-showdown-shiki/pull/40)

Having an option to enable this behaviour in the transformer would be useful.

lachieh commented 2 months ago

I'd also make the vote to make this the default behavior with the option to turn it off. This would obviously be a breaking change, but it's hitting more than a few projects.