test(theme-common): Add tests for getLineNumbersStart

[Danielku15]

Pre-flight checklist

• I have read the Contributing Guidelines on pull requests.
• If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
• If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

This PR is part of a series related to #11008 and it superceeds #11011 in the goal to splitup the work into more isolated refactoring/feature parts.

Step 1 (this PR): Splitup current parseLines logic
Step 2 (#11019): Parse metastring into metaOptions and use it across components.
Step 3 (#11021): Add parsing of any options specified in the metastring.
Step 4 (#11022): Add CodeBlockToken component to customize DOM of highlighted tokens.
Step 5 (#11023): Pass metaOptions to whole codeblock component tree.

The motivation of this part is was to prepare the codebase for the upcoming extensions and adaptions. Initially step 2 had the parsing added to parseLines so splitting things up and defining types was a good preparation for extension.

With the final version of all changes, parseLines is not used for this purpose. I still kept this PR for two purposes:

1. The splitup and type definitions might make some things easier to maintain.
2. The additional tests are ensuring correct behavior of the nest steps.

Test Plan

The code change aims to have 0 functional impact and is therefore covered by the current unit/integration tests. These tests are green.

Manual testing by checking /docs/markdown-features/code-blocks showed no impact of the change.

Additional tests for getLineNumbersStart were added to avoid regression bugs in upcoming PRs. more tests likely come in every step.

Test links

Deploy preview: https://deploy-preview-11017--docusaurus-2.netlify.app
Code Blocks: https://deploy-preview-11017--docusaurus-2.netlify.app/docs/markdown-features/code-blocks/

Related issues/PRs

• feat(theme-common, theme-classic): Improve CodeBlock Extensibility #11011
• Improve CodeBlock Extensibility #11008

[netlify]

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	9e842ae
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/67e690439b280a0008629e9d
😎 Deploy Preview	https://deploy-preview-11017--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[github-actions]

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	Report
/	🟠 64	🟢 98	🟢 100	🟢 100	Report
/docs/installation	🔴 49	🟢 97	🟢 100	🟢 100	Report
/docs/category/getting-started	🟠 72	🟢 100	🟢 100	🟠 86	Report
/blog	🟠 62	🟢 96	🟢 100	🟠 86	Report
/blog/preparing-your-site-for-docusaurus-v3	🟠 62	🟢 92	🟢 100	🟢 100	Report
/blog/tags/release	🟠 63	🟢 96	🟢 100	🟠 86	Report
/blog/tags	🟠 73	🟢 100	🟢 100	🟠 86	Report

[slorber]

LGTM but please use functional style code like before, and don't mutate function inputs

[Danielku15]

I decided to revert my code changes. Looking at the remarks you rather prefer to stay on the old function implementation as it was. While your arguments might be valid, I'm not sure if I can invest so much time in satisfying all your wishes on code style, hence I'm rather removing changes not focusing on the direct goal of the new feature.

[slorber]

I decided to revert my code changes.

Some revert was not strictly needed, I'm fine if we extract option/return types for example

Looking at the remarks you rather prefer to stay on the old function implementation as it was.

No I think it's fine to split the logic in 2 parts as you did, but useless side effects like this are a common anti-pattern I'd like to avoid. Functions should be self-encapsulated. I'll see if I can add an eslint to forbid this style by default.

While your arguments might be valid, I'm not sure if I can invest so much time in satisfying all your wishes on code style, hence I'm rather removing changes not focusing on the direct goal of the new feature.

As you want, I can as well refactor it myself.

External contributor PRs also take time to review, and often it's faster if you tell me what you want exactly, and I implement it myself. Only invest time if you care about contributing (and agree to meet project standards to do so), and/or want to be progressively onboarded as a regular contributor.

[Danielku15]

decided to revert my code changes.

Some revert was not strictly needed, I'm fine if we extract option/return types for example

Considering the motivation and focus of my PRs it somehow has become out-of-scope for me. Also the added value is fairly limited as the related types are purely for internal use. So I went the simple path here of reverting it fully.

If its really a valueable change for the project it could become an own PR though.

Looking at the remarks you rather prefer to stay on the old function implementation as it was.

No I think it's fine to split the logic in 2 parts as you did, but useless side effects like this are a common anti-pattern I'd like to avoid. Functions should be self-encapsulated. I'll see if I can add an eslint to forbid this style by default.

I personally do not fully agree on this statement, but only as it might be a bit over-generalizing. Not every function on every level has to be pure, I'd rather consider that a anti-pattern as it should be a proper case-by-case decision. In this specific scenario:

• The related functions are not exposed and an internal implementation detail. They purely serve the aspect of reducing code complexity int the particular function. In programming languages with explicit inlining, you'd likely even mark these as inline hinting the runtime/compiler that there is no point in handling them as own stack-frame.
• Enforcing only pure-functions / immutable structures can lead to performance degredations. Especially in JavaScript where all objects are stored on the heap and every copy has to be garbage collected again. Handling every modification as mutated copy easily produces a lot of waste.

Of course it might be possible to refactor things even further here to have only pure functions even for the sub-parts of the exposed function. For my planned contribution its simply out of scope to invest so much energy in refactoring those (fairly insignificant) bits as they do not contribute to the motivation and goals of the PR-series. 😉

While your arguments might be valid, I'm not sure if I can invest so much time in satisfying all your wishes on code style, hence I'm rather removing changes not focusing on the direct goal of the new feature.

As you want, I can as well refactor it myself.

The commit hashes are still linked in the PR history and AFAIK GitHub will not clean them. Feel free to cherry-pick any changes in this area into a new PR 😁

External contributor PRs also take time to review, and often it's faster if you tell me what you want exactly, and I implement it myself. Only invest time if you care about contributing (and agree to meet project standards to do so), and/or want to be progressively onboarded as a regular contributor.

Sure it takes time, and external contributors have to take higher hurdles than direct team members. External contributors have to take typically higher hurdles than core members and that's not bad in itself. At the end of the day the maintainers have to live with the code they pull in and maintain it further so it's only natural to have some higher expectations.

Many good articles exist around the matter of healthy code reviews and why they are hard to do, and there is no "correct" way. 😁

I have no hard feelings in reworking or reverting changes until some extend. On my general needs things might get a bit clearer across the PRs, but talking in "solution space" and not in "requirements":

1. I want to swizzle all levels of the code blocks "CodeBlock > CodeBlockLine > CodeBlockToken (new component)" to customize rendering.
2. In the swizzled components (all levels) I need the options contained in the metastring available for customization.

[slorber]

Thanks

I guess we won't easily agree on everything, but I can probably refactor things in a way I want that matches your goal

Will look at this later, but we can already merge this PR

[slorber]

Going to close the current PRs as I did refactor and made the whole CodeBlock thing easier to swizzle and customize.

See:

• refactor(theme): refactor CodeBlock parseLines logic + use inline snapshots to ease review #11058
• refactor(theme): CodeBlock, centralize metadata parsing + refactor theme component #11059
• refactor(theme): introduce CodeBlockContextProvider + split into smaller components #11062

You can use this hook anywhere in the code block tree to get useful data/apis:

import {useCodeBlockContext} from '@docusaurus/theme-common/internal';

const {metadata, wordWrap} = useCodeBlockContext()

I'll refactor the live code block next week

[slorber]

The Live Code Block component is being refactored here: #11077

It is being split into many smaller components, easier to swizzle and customize.

---

The original and live code blocks components have not much in common so to me it didn't really make sense to build an abstraction over both.

The metastring of one is totally different from the metastring of the other, so I don't think it makes sense to use a shared type for both for example.

For now the live code block keeps metastring parsing as is. We only need live and noInline attributes.

I'll see later how to eventually parse the metastring with a remark plugin once we have better global markdown plugin lifecycles (related to #5999)