Skip to content

Commit 2d8d267

Browse files
authored
fix(mdxish): combine code tabs if separated by CLRF token \r\n (#1372)
[![PR App][icn]][demo] | Fix CX-3027 :-------------------:|:----------: ## 🧰 Changes While investigating [CX-3027](https://linear.app/readme-io/issue/CX-3027/multi-parameter-set-codetabs-render-as-separate-components-for), I found that one of the cause of the code tabs separating was because the raw document had CLRF new line tokens, so \r\n instead of only \n in between the code blocks. I verified that this resulted in separate code blocks in mdxish rendering, but seems like it's correctly tabbed in legacy from looking at the resulting mdast trees. The adjacency check in the code-tabs transformer only accounted for LF (\n) line endings. CRLF adds an extra byte (\r) to the offset gap, causing the check to fail and rendering each code block separately. So I updated the check to also accept CRLF by allowing a gap of start.column + 1 when the blocks are still on consecutive lines (i.e., the extra byte is \r, not a blank line)/ ## 🧬 QA & Testing Unfortunately, it's quite hard to visually test in an actual doc because adding in the \r\n in the raw doc will render it literally, and I read that this might be a Windows os specific thing & not Mac. The best way I can think of is through the jest tests I created, which includes some regression tests. One regression check is to ensure the code blocks behaviour should follow MDX & legacy, so testing multiple consecutive blocks, adding some space / characters between the blocks, adding more than 1 line space between, etc. - [Broken on production][prod]. - [Working in this PR app][demo]. [demo]: https://markdown-pr-PR_NUMBER.herokuapp.com [prod]: https://SUBDOMAIN.readme.io [icn]: https://user-images.githubusercontent.com/886627/160426047-1bee9488-305a-4145-bb2b-09d8b757d38a.svg
1 parent b8d9e4c commit 2d8d267

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

__tests__/lib/render-mdxish/CodeTabs.test.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,28 @@ ${pythonCode}
4949
expect(buttons[0]).toHaveTextContent('C++');
5050
expect(buttons[1]).toHaveTextContent('Python');
5151
});
52+
53+
it('should render code tabs when there is a slash r (CRLF) character between the backticks', () => {
54+
const mdSlash = '```python\nprint("hello")\n```\r\n```javascript\nconsole.log("hello")\n```';
55+
const modSlash = renderMdxish(mdxish(mdSlash));
56+
const { container } = render(<modSlash.default />);
57+
58+
expect(container.querySelector('div.CodeTabs')).toBeInTheDocument();
59+
expect(container.querySelectorAll('div.CodeTabs_initial')).toHaveLength(1);
60+
61+
const buttons = container.querySelectorAll('button');
62+
expect(buttons).toHaveLength(2);
63+
expect(buttons[0]).toHaveTextContent('Python');
64+
expect(buttons[1]).toHaveTextContent('JavaScript');
65+
});
66+
67+
it('should render code tabs even when there are spaces after the first code block', () => {
68+
const mdSpace = '```python\nprint("hello")\n``` \n```javascript\nconsole.log("hello")\n```';
69+
const modSpace = renderMdxish(mdxish(mdSpace));
70+
const { container } = render(<modSpace.default />);
71+
expect(container.querySelector('div.CodeTabs')).toBeInTheDocument();
72+
expect(container.querySelectorAll('div.CodeTabs_initial')).toHaveLength(1);
73+
});
5274
});
5375

5476
describe('given a mermaid diagram', () => {
@@ -161,4 +183,27 @@ ${mermaidCode}
161183
expect(buttons[1]).toHaveTextContent('Python');
162184
});
163185
});
186+
187+
describe('given non consecutive code blocks', () => {
188+
it('should not combine 2 code blocks that are separated by 2 empty lines', () => {
189+
const md = '```python\nprint("hello")\n```\n\n```javascript\nconsole.log("hello")\n```';
190+
const mod = renderMdxish(mdxish(md));
191+
const { container } = render(<mod.default />);
192+
expect(container.querySelectorAll('div.CodeTabs_initial')).toHaveLength(2);
193+
});
194+
195+
it('should not combine 2 code blocks that are separated by 2 empty lines with \\r\\n', () => {
196+
const md = '```python\r\nprint("hello")\n```\r\n\r\n```javascript\nconsole.log("hello")\r\n```';
197+
const mod = renderMdxish(mdxish(md));
198+
const { container } = render(<mod.default />);
199+
expect(container.querySelectorAll('div.CodeTabs_initial')).toHaveLength(2);
200+
});
201+
202+
it('should not combine 2 code blocks that are separated by a new line and text before the code block', () => {
203+
const md = '```python\nprint("hello")\n```\nhello```javascript\nconsole.log("hello")\n```';
204+
const mod = renderMdxish(mdxish(md));
205+
const { container } = render(<mod.default />);
206+
expect(container.querySelectorAll('div.CodeTabs_initial')).toHaveLength(1);
207+
});
208+
});
164209
});

processor/transform/code-tabs.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,18 @@ const codeTabsTransformer =
2828
const sibling = parent.children[walker];
2929
if (!isCode(sibling)) break;
3030

31+
// Check that the two code blocks are truly adjacent (no blank lines or
32+
// other content between them). The `gap` is the number of raw characters
33+
// between the end of the previous block and the start of this one.
34+
// For a LF-separated pair the gap equals `start.column` (the newline
35+
// char(s) plus any indentation). CRLF line endings add one extra byte
36+
// (\r) without advancing the line count, so we also accept
37+
// `start.column + 1` provided the blocks are still on consecutive lines.
3138
const olderSibling = parent.children[walker - 1];
32-
if (olderSibling.position.end.offset + sibling.position.start.column !== sibling.position.start.offset) break;
39+
const gap = sibling.position.start.offset - olderSibling.position.end.offset;
40+
const lineDiff = sibling.position.start.line - olderSibling.position.end.line;
41+
const isCRLF = gap === sibling.position.start.column + 1 && lineDiff === 1;
42+
if (gap !== sibling.position.start.column && !isCRLF) break;
3343

3444
children.push(sibling);
3545
// eslint-disable-next-line no-plusplus

0 commit comments

Comments
 (0)