shikijs / twoslash

You take some Shiki, add a hint of TypeScript compiler, and 🎉 incredible static code samples
https://shikijs.github.io/twoslash/
MIT License
1.09k stars 53 forks source link

Highlighting multiple specific lines offset by 1 #154

Open grikomsn opened 2 years ago

grikomsn commented 2 years ago

Hi there, been a huge fan of twoslash!

Currently experimenting with remark-shiki-twoslash on my Next.js + @mdx-js/mdx v2 project and for some reason when highlighting multiple specific lines (e.g. {7,12}) it offsets by one line. No custom configurations and no custom stylesheets (only using provided from readme), only using rehype-raw to fix #125.

Take for example, this snippet (taken here):

```jsx {7,12}
function CustomLink(props) {
  const isRelative = props.href?.startsWith("/") ?? false;

  if (isRelative) {
    return (
      <Link href={props.href}>
        <a {...props} />
      </Link>
    );
  }

  return <a {...props} target="_blank" />;
}
```

Produces this (using remark-shiki-twoslash):

image

Which it not correct and should produce this (using rehype-pretty-code):

image

My guess it's somewhere around this line but I'll try experimenting if it's from my end of from shiki-twoslash's end:

https://github.com/shikijs/twoslash/blob/d5510d7e589a4d53e40c8c34ade7bbe0826fdb55/packages/shiki-twoslash/src/renderers/twoslash.ts#L71

Anything I did wrong? I'd love to make a reproducible project for this particular case if anyone's interested.

orta commented 2 years ago

:wave: - I might not accept a PR changing this, unless you can prove really is a bug and not just that you expected rehype-pretty-code's style and got ours instead. Because I probably spent some time thinking about whether we want to be 0 based of 1 based with the line settings and churn here is extremely expensive for everyone.

orta commented 2 years ago

Yep, confirming this is intentional to match editors which start with line 1 and gatsby-remark-vscode which we share infra with

I'm not against having a config setting like highlightsUseZeroIndexing: true which can configure this to help migrations across tools.

jacob-8 commented 1 year ago

@orta - Can you please add a little more documentation at least to the shiki-twoslash readme about the line highlighting discrepancy between twoslash mode and not using twoslash. As seen from your playground, when using the twoslash meta attribute, highlighting is line 1 based: image

And without it is line 0 based: image

This is due to the above mentioned difference between the defaultShikiRenderer: https://github.com/shikijs/twoslash/blob/454c1b7ece7b05f669c61af4b4a535475b2adbab/packages/shiki-twoslash/src/renderers/shiki.ts#L27

and the twoslashRenderer https://github.com/shikijs/twoslash/blob/454c1b7ece7b05f669c61af4b4a535475b2adbab/packages/shiki-twoslash/src/renderers/twoslash.ts#L71

That was a little confusing for me. I'm going to circumvent it at the moment by just subtracting 1 from line numbers when the twoslash meta is not provided. (to me starting at 1 makes the most sense because that matches my code editor, but it seems others think 0 is the more corrent?... :)

I love this tool by the way! I'm very close to publishing an mdsvex-shiki-twoslash plugin that you can use as a highlighter in MDSvex. (MDSvex sadly doesn't play nice with shiki as a remark plugin at the moment).

orta commented 1 year ago

Sounds more like a bug to me, they should definitely be consistent 👍🏻