feat: allow matching region end in snippets without tag (#4287)

Co-authored-by: Divyansh Singh <40380293+brc-dd@users.noreply.github.com>

__tests__/unit/node/markdown/plugins/snippet.test.ts

@@ -1,4 +1,9 @@
-import { dedent, rawPathToToken } from 'node/markdown/plugins/snippet'
+import {
+  dedent,
+  findRegion,
+  rawPathToToken
+} from 'node/markdown/plugins/snippet'
+import { expect } from 'vitest'
 
 const removeEmptyKeys = <T extends Record<string, unknown>>(obj: T) => {
   return Object.fromEntries(
@@ -94,9 +99,228 @@ describe('node/markdown/plugins/snippet', () => {
     })
   })
 
-  test('rawPathToToken', () => {
+  describe('rawPathToToken', () => {
-    rawPathTokenMap.forEach(([rawPath, token]) => {
+    test.each(rawPathTokenMap)('%s', (rawPath, token) => {
       expect(removeEmptyKeys(rawPathToToken(rawPath))).toEqual(token)
     })
   })
+
+  describe('findRegion', () => {
+    it('returns null when no region markers are present', () => {
+      const lines = ['function foo() {', '  console.log("hello");', '}']
+      expect(findRegion(lines, 'foo')).toBeNull()
+    })
+
+    it('ignores non-matching region names', () => {
+      const lines = [
+        '// #region regionA',
+        'some code here',
+        '// #endregion regionA'
+      ]
+      expect(findRegion(lines, 'regionC')).toBeNull()
+    })
+
+    it('returns null if a region start marker exists without a matching end marker', () => {
+      const lines = [
+        '// #region missingEnd',
+        'console.log("inside region");',
+        'console.log("still inside");'
+      ]
+      expect(findRegion(lines, 'missingEnd')).toBeNull()
+    })
+
+    it('returns null if an end marker exists without a preceding start marker', () => {
+      const lines = [
+        '// #endregion ghostRegion',
+        'console.log("stray end marker");'
+      ]
+      expect(findRegion(lines, 'ghostRegion')).toBeNull()
+    })
+
+    it('detects C#/JavaScript style region markers with matching tags', () => {
+      const lines = [
+        'Console.WriteLine("Before region");',
+        '#region hello',
+        'Console.WriteLine("Hello, World!");',
+        '#endregion hello',
+        'Console.WriteLine("After region");'
+      ]
+      const result = findRegion(lines, 'hello')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          'Console.WriteLine("Hello, World!");'
+        )
+      }
+    })
+
+    it('detects region markers even when the end marker omits the region name', () => {
+      const lines = [
+        'Console.WriteLine("Before region");',
+        '#region hello',
+        'Console.WriteLine("Hello, World!");',
+        '#endregion',
+        'Console.WriteLine("After region");'
+      ]
+      const result = findRegion(lines, 'hello')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          'Console.WriteLine("Hello, World!");'
+        )
+      }
+    })
+
+    it('handles indented region markers correctly', () => {
+      const lines = [
+        '  Console.WriteLine("Before region");',
+        '  #region hello',
+        '  Console.WriteLine("Hello, World!");',
+        '  #endregion hello',
+        '  Console.WriteLine("After region");'
+      ]
+      const result = findRegion(lines, 'hello')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          '  Console.WriteLine("Hello, World!");'
+        )
+      }
+    })
+
+    it('detects TypeScript style region markers', () => {
+      const lines = [
+        'let regexp: RegExp[] = [];',
+        '// #region foo',
+        'let start = -1;',
+        '// #endregion foo'
+      ]
+      const result = findRegion(lines, 'foo')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          'let start = -1;'
+        )
+      }
+    })
+
+    it('detects CSS style region markers', () => {
+      const lines = [
+        '.body-content {',
+        '/* #region foo */',
+        '  padding-left: 15px;',
+        '/* #endregion foo */',
+        '  padding-right: 15px;',
+        '}'
+      ]
+      const result = findRegion(lines, 'foo')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          '  padding-left: 15px;'
+        )
+      }
+    })
+
+    it('detects HTML style region markers', () => {
+      const lines = [
+        '<div>Some content</div>',
+        '<!-- #region foo -->',
+        '  <h1>Hello world</h1>',
+        '<!-- #endregion foo -->',
+        '<div>Other content</div>'
+      ]
+      const result = findRegion(lines, 'foo')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          '  <h1>Hello world</h1>'
+        )
+      }
+    })
+
+    it('detects Visual Basic style region markers (with case-insensitive "End")', () => {
+      const lines = [
+        'Console.WriteLine("VB")',
+        '#Region VBRegion',
+        '  Console.WriteLine("Inside region")',
+        '#End Region VBRegion',
+        'Console.WriteLine("Done")'
+      ]
+      const result = findRegion(lines, 'VBRegion')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          '  Console.WriteLine("Inside region")'
+        )
+      }
+    })
+
+    it('detects Bat style region markers', () => {
+      const lines = ['::#region foo', 'echo off', '::#endregion foo']
+      const result = findRegion(lines, 'foo')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          'echo off'
+        )
+      }
+    })
+
+    it('detects C/C++ style region markers using #pragma', () => {
+      const lines = [
+        '#pragma region foo',
+        'int a = 1;',
+        '#pragma endregion foo'
+      ]
+      const result = findRegion(lines, 'foo')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          'int a = 1;'
+        )
+      }
+    })
+
+    it('returns the first complete region when multiple regions exist', () => {
+      const lines = [
+        '// #region foo',
+        'first region content',
+        '// #endregion foo',
+        '// #region foo',
+        'second region content',
+        '// #endregion foo'
+      ]
+      const result = findRegion(lines, 'foo')
+      expect(result).not.toBeNull()
+      if (result) {
+        expect(lines.slice(result.start, result.end).join('\n')).toBe(
+          'first region content'
+        )
+      }
+    })
+
+    it('handles nested regions with different names properly', () => {
+      const lines = [
+        '// #region foo',
+        "console.log('line before nested');",
+        '// #region bar',
+        "console.log('nested content');",
+        '// #endregion bar',
+        '// #endregion foo'
+      ]
+      const result = findRegion(lines, 'foo')
+      expect(result).not.toBeNull()
+      if (result) {
+        const extracted = lines.slice(result.start, result.end).join('\n')
+        const expected = [
+          "console.log('line before nested');",
+          '// #region bar',
+          "console.log('nested content');",
+          '// #endregion bar'
+        ].join('\n')
+        expect(extracted).toBe(expected)
+      }
+    })
+  })
 })

src/node/markdown/plugins/snippet.ts

@@ -51,47 +51,71 @@ export function dedent(text: string): string {
   return text
 }
 
-function testLine(
-  line: string,
-  regexp: RegExp,
-  regionName: string,
-  end: boolean = false
-) {
-  const [full, tag, name] = regexp.exec(line.trim()) || []
-
-  return (
-    full &&
-    tag &&
-    name === regionName &&
-    tag.match(end ? /^[Ee]nd ?[rR]egion$/ : /^[rR]egion$/)
-  )
-}
-
 export function findRegion(lines: Array<string>, regionName: string) {
-  const regionRegexps = [
+  const regionRegexps: [RegExp, RegExp][] = [
-    /^\/\/ ?#?((?:end)?region) ([\w*-]+)$/, // javascript, typescript, java
+    [
-    /^\/\* ?#((?:end)?region) ([\w*-]+) ?\*\/$/, // css, less, scss
+      /^[ \t]*\/\/ ?#?(region) ([\w*-]+)$/,
-    /^#pragma ((?:end)?region) ([\w*-]+)$/, // C, C++
+      /^[ \t]*\/\/ ?#?(endregion) ?([\w*-]*)$/
-    /^<!-- #?((?:end)?region) ([\w*-]+) -->$/, // HTML, markdown
+    ], // javascript, typescript, java
-    /^#((?:End )Region) ([\w*-]+)$/, // Visual Basic
+    [
-    /^::#((?:end)region) ([\w*-]+)$/, // Bat
+      /^\/\* ?#(region) ([\w*-]+) ?\*\/$/,
-    /^# ?((?:end)?region) ([\w*-]+)$/ // C#, PHP, Powershell, Python, perl & misc
+      /^\/\* ?#(endregion) ?([\w*-]*) ?\*\/$/
+    ], // css, less, scss
+    [/^#pragma (region) ([\w*-]+)$/, /^#pragma (endregion) ?([\w*-]*)$/], // C, C++
+    [/^<!-- #?(region) ([\w*-]+) -->$/, /^<!-- #?(endregion) ?([\w*-]*) -->$/], // HTML, markdown
+    [/^[ \t]*#(Region) ([\w*-]+)$/, /^[ \t]*#(End Region) ?([\w*-]*)$/], // Visual Basic
+    [/^::#(region) ([\w*-]+)$/, /^::#(endregion) ?([\w*-]*)$/], // Bat
+    [/^[ \t]*# ?(region) ([\w*-]+)$/, /^[ \t]*# ?(endregion) ?([\w*-]*)$/] // C#, PHP, Powershell, Python, perl & misc
   ]
 
-  let regexp = null
+  let chosenRegex: [RegExp, RegExp] | null = null
-  let start = -1
+  let startLine = -1
-
+  // find the regex pair for a start marker that matches the given region name
-  for (const [lineId, line] of lines.entries()) {
+  for (let i = 0; i < lines.length; i++) {
-    if (regexp === null) {
+    const line = lines[i].trim()
-      for (const reg of regionRegexps) {
+    for (const [startRegex, endRegex] of regionRegexps) {
-        if (testLine(line, reg, regionName)) {
+      const startMatch = startRegex.exec(line)
-          start = lineId + 1
+      if (
-          regexp = reg
+        startMatch &&
+        startMatch[2] === regionName &&
+        /^[rR]egion$/.test(startMatch[1])
+      ) {
+        chosenRegex = [startRegex, endRegex]
+        startLine = i + 1
         break
       }
     }
-    } else if (testLine(line, regexp, regionName, true)) {
+    if (chosenRegex) break
-      return { start, end: lineId, regexp }
+  }
+  if (!chosenRegex) return null
+
+  const [startRegex, endRegex] = chosenRegex
+  let counter = 1
+  // scan the rest of the lines to find the matching end marker, handling nested markers
+  for (let i = startLine; i < lines.length; i++) {
+    const trimmed = lines[i].trim()
+    // check for an inner start marker for the same region
+    const startMatch = startRegex.exec(trimmed)
+    if (
+      startMatch &&
+      startMatch[2] === regionName &&
+      /^[rR]egion$/.test(startMatch[1])
+    ) {
+      counter++
+      continue
+    }
+    // check for an end marker for the same region
+    const endMatch = endRegex.exec(trimmed)
+    if (
+      endMatch &&
+      // allow empty region name on the end marker as a fallback
+      (endMatch[2] === regionName || endMatch[2] === '') &&
+      /^[Ee]nd ?[rR]egion$/.test(endMatch[1])
+    ) {
+      counter--
+      if (counter === 0) {
+        return { start: startLine, end: i, regexp: chosenRegex }
+      }
     }
   }
 
@@ -181,7 +205,13 @@ export const snippetPlugin = (md: MarkdownIt, srcDir: string) => {
         content = dedent(
           lines
             .slice(region.start, region.end)
-            .filter((line) => !region.regexp.test(line.trim()))
+            .filter((line) => {
+              const trimmed = line.trim()
+              return (
+                !region.regexp[0].test(trimmed) &&
+                !region.regexp[1].test(trimmed)
+              )
+            })
             .join('\n')
         )
       }