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

Highlighting multiple specific lines offset by 1 #154

Open
grikomsn opened this issue May 22, 2022 · 4 comments
Open

Highlighting multiple specific lines offset by 1 #154

grikomsn opened this issue May 22, 2022 · 4 comments
Labels
Enhancement New feature or request You can do this The idea is well specified and good for a PR

Comments

@grikomsn
Copy link

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:

const hiClass = hasHighlight ? (hl(i + 1) ? " highlight" : " dim") : ""

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

@orta
Copy link
Contributor

orta commented May 23, 2022

👋 - 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 orta added Enhancement New feature or request You can do this The idea is well specified and good for a PR labels May 24, 2022
@orta
Copy link
Contributor

orta commented May 24, 2022

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
Copy link

jacob-8 commented Dec 23, 2022

@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:

const hiClass = hasHighlight ? (hl(i) ? " highlight" : " dim") : ""

and the twoslashRenderer

const hiClass = hasHighlight ? (hl(i + 1) ? " highlight" : " dim") : ""

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
Copy link
Contributor

orta commented Dec 23, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request You can do this The idea is well specified and good for a PR
Projects
None yet
Development

No branches or pull requests

3 participants