Use diff syntax for codeblocks

[cortinico]

I've updated several codeblocks in the migration guide to use the diff syntax + I've added several missing title='...' so it's cleared of which file we're talking about.

[netlify]

✔️ Deploy Preview for react-native ready!

🔨 Explore the source changes: ac2ad82

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-native/deploys/621e63c5ba2050000807f08f

😎 Browse the preview: https://deploy-preview-2984--react-native.netlify.app

[netlify]

✔️ Deploy Preview for react-native ready!

🔨 Explore the source changes: 558df33

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-native/deploys/621e63d0645219000788ac32

😎 Browse the preview: https://deploy-preview-2984--react-native.netlify.app

[Simek]

Most of the changes looks good, thank you for working on this! 👍

However I don't like that much, how the diff highlight works. I see two problems with it:

• manually selecting and copying lines or using "Copy" button now includes + or - chars for diff lines, which had to be manually removed after paste,
• the language based code highlight do not work in diff blocks (maybe this can be added somehow?), which leads to decreased readability of the larger example code blocks.

Unfortunately there is no easy solution to fix those issues, and there are not many viable alternatives (there is // highlight-start/end but it's just a visual lines highlight, no ability to alter the appearance or leave a comment - https://docusaurus.io/docs/next/markdown-features/code-blocks#line-highlighting), so I think it's fine to merge it as is for now, but we should investigate, how we can improve diff code block rendering in the future. Also, maybe part of this can be addressed directly via PR to Docusaurus, I will look into that.

[cortinico]

which had to be manually removed after paste,

Yup I noticed the same. What is worse is that also selecting multiple lines carries over the +.

(there is // highlight-start/end but it's just a visual lines highlight, no ability to alter the appearance or leave a comment - docusaurus.io/docs/next/markdown-features/code-blocks#line-highlighting)

Maybe I can update those diff which are involving more than say 5 lines, to use this feature instead, wdyt?

Also, maybe part of this can be addressed directly via PR to Docusaurus, I will look into that.

And yes, if we fix this, we should definitely backport them upstream

[Simek]

It looks like Prism supports diff highlight with language highlight included via plugin, not sure if react-prism-renderer, which is used in Docusaurus under the hood supports that:

• https://prismjs.com/plugins/diff-highlight/

Maybe I can update those diff which are involving more than say 5 lines, to use this feature instead, wdyt?

I have no strong opinion about this, we can leave the diff blocks as is or later the bigger ones as you suggested. I think if we want to try improve this in the future, it would be nice to avoid the work which will be reverted if we succeed. Also, in the other hand using diffs there also improves the examples i na way that it shifts the focus to the important part of the snippets.

[slorber]

On the docusaurus side, we'd definitively be interested to be able to:

• have syntax highlighting on diff codeblocks
• make it easier to copy working code from diff

We had a few issues in our own tutorials where users suggested to move back from diff to regular code block:

• docs: make tutorial code block directly copyable docusaurus#6833
• Copy control in the getting-started tutorial copies confusing code docusaurus#4698

cc @Josh-Cena

[Josh-Cena]

have syntax highlighting on diff codeblocks

This can be done with a Prism plugin. I tried a while back by swizzling prism-include-languages but no luck, and didn't bother making it work.

make it easier to copy working code from diff

This would be done through a string sanitization before passing to copyToClipboard, but last time when facebook/docusaurus#4698 was brought up, we decided that if a diff code block needs to be copied, it should probably be a normal code block instead. The valid use-cases of diff are those that are purely illustrative, like https://docusaurus.io/docs/sidebar/autogenerated#using-number-prefixes

Not sure if we necessarily want to handle diff specially—should we copy the - lines, or only the + lines? I mean, the entire purpose of diff is to show a process, not a result...

[slorber]

Not sure if we necessarily want to handle diff specially—should we copy the - lines, or only the + lines? I mean, the entire purpose of diff is to show a process, not a result...

Somehow agree but users expect this to work apparently. Also there's a copy button on :hover so it's a bit misleading. Maybe we should just remove that copy button for diff code blocks?

[Josh-Cena]

Maybe we should just remove that copy button for diff code blocks?

Doesn't sound like a bad idea.

[cortinico]

What's the final decision here @Simek ? How do you feel about using the diff syntax only to changes of 3/4 lines?

[Simek]

Sound good to me! 👍 It's not a blocker anyway, since there is no ideal solution yet.

P.S. Unfortunately I was not able to look at the Prisma plugin port/addition yet, but I have this in my schedule for this month.

[Josh-Cena]

@Simek Recently I've come to known that the Prism plugin actually won't work with prism-react-renderer which Docusaurus uses. See FormidableLabs/prism-react-renderer#90

[Josh-Cena]

I've recently set this up for the TS-ESLint website: https://deploy-preview-5099--typescript-eslint.netlify.app/docs/linting/monorepo#one-tsconfigjson-per-package-and-an-optional-one-in-the-root

Had to swizzle CodeBlock to achieve the copy button customization, not sure if we can do better in Docusaurus

[Simek]

Thanks for the hint, I will look at the code in there soon and try to port it into RN website.

[Josh-Cena]

I think Docusaurus can make the swizzling a bit less heavy here: e.g. we can pass lineClassNames into CopyButton so that we only need to swizzle CopyButton to change its behavior. @slorber WDYT?

[slorber]

@Josh-Cena as far as I understand you talk about this PR: https://github.com/typescript-eslint/typescript-eslint/pull/5099/files

Not against passing more things to <CopyButton> if that helps reduce the amount of swizzled code

However, maybe it would be better to make this more automatic in the end: using diff should just work 🤷‍♂️

[Josh-Cena]

IDK—I feel like this is quite opinionated and not necessarily correct, and it would be dangerous for us on a framework level to assume what users want when they use diff—what if they just want to illustrate, well, a diff?

In typescript-eslint/typescript-eslint#5099 I made the removed lines non-selectable and non-copyable, which suits that particular use-case, but is not a fair assumption to make.

[slorber]

@Josh-Cena I don't think many wouldn't want this behavior in practice

What if we made diff behavior opinionated automatically, but allow another metastring to opt-out?

Like diff preserve vs diff?

This requires to document it, but that seems fine

[Josh-Cena]

I'm ok with that, sure... If we can design it well.

[cortinico]

I'm abandoning this. The docs changed so much that this is not relevant. We've been using the Diff blocks sporadically in the new docs + we reduce the amount of changes needed for the new arch so this is probably not relevant anymore.