Finding niche bugs, and fixing them
Posted on:
Found a bug in a rehype plugin. Let's dive into what happened and how I fixed it.
Written by: David Ward
Summary
Sometimes, it’s just the small things that add up to make things better for everyone.
The project
I stumbled across a rehype plugin for giving a little bit more power to code blocks, like line highlighting and custom line counts, called rehype-pretty-code.
You can see a demo and look at the docs on their deployed app.
The feature in question
It recently got a contribution that expanded upon its current feature set for setting the starting line number when you request showing line numbers, like so:
```js showLineNumbers{3}
const test = 1;
const test2 = 2;
```Which when rendered, looks like so:
const test = 1;
const test2 = 2;You’ll notice that it starts on the line 3 when beginning the code block. Great!
It also has an ability to highlight a specific line number as well, like so:
```js {1}
const test = 1;
const test2 = 2;
```Which when rendered, looks like so:
const test = 1;
const test2 = 2;You’ll notice that the first line is highlighted in the browser. Great!
The problem
Something I noticed was when using both of these features at the same time, depending on how you authored your markdown, you may experience issues with line highlighting not respecting the number you set.
Example 1: showLineNumbers followed by {x}
For example, when using showLineNumbers followed by {x}:
```js showLineNumbers{3} {2}
const test = 1;
const test2 = 2;
```Which when rendered, looks like so:
const test = 1;
const test2 = 2;You will observe the bug: the second line (labeled `4) is not highlighted, as would be expected.
Note: You will not observe the bug once the fix is in place.
Example 2: {x} followed by showLineNumbers
```js {2} showLineNumbers{3}
const test = 1;
const test2 = 2;
```Which when rendered, looks like so:
const test = 1;
const test2 = 2;You will not observe the bug: the second line (labeled `4) is not highlighted, as would be expected.
The bug in the code
I found where the issue was: There was a regex that was a little too greedy and not taking into account the newly added showLineNumbers feature:
const lineNumbers = meta ? rangeParser(meta.match(/{(.*)}/)?.[1] ?? "") : [];The fix
So how do we fix it? Well, the simplest way is that since there are no other options that use the {} characters, we look to see if the character before a { is not a whitespace, and if so we ignore that group and keep searching.
const lineNumbers = meta
? rangeParser(meta.match(/(?:^|\s){(.*?)/)?.[1] ?? "")
: [];The addition of the (?:^|\s) and adding a ? in the capture group is the key part!
Adding tests
But we wouldn’t be a good dev or tester if we didn’t also add tests that cover this!
Turns out they are using snapshot testing, which accepts a markdown file as fixture input, and expected results with the same name, but in an html file, corresponding to what html the plugin would generate exactly.
So, we added a markdown file with a very similar set of data as we showed in the problem, and then used one of the existing results html as a template and modified data according to what the markdown contained…
And the tests passed!
Impact and severity
This is a pretty minor bug in the grand scheme of things, with rather low severity, but still a bug and the fix wasn’t terribly hard, either.
Always strive for quality, and leave things better than you found them.