Repo sync

content/contributing/writing-for-github-docs/creating-screenshots.md

@@ -130,7 +130,7 @@ If the primary goal in showing a dropdown menu is to help the reader distinguish
 
 ## Highlighting elements in screenshots
 
-To highlight a specific UI element in a screenshot, use our special theme for [Snagit](https://www.techsmith.com/screen-capture.html) to apply a contrasting stroke around the element.
+To highlight a specific UI element in a screenshot, use our special theme for [Snagit](https://www.techsmith.com/snagit/) to apply a contrasting stroke around the element.
 
 The stroke is the color `fg.severe` in the [Primer Design System](https://primer.style/design/) (HEX #BC4C00 or RGB 188, 76, 0). This dark orange has good color contrast on both white and black. To check contrast on other background colors, use the [Color Contrast Analyzer](https://www.tpgi.com/color-contrast-checker/).
 

src/content-render/tests/link-error-line-numbers.js

@@ -0,0 +1,154 @@
+import { describe, expect, test, beforeEach, afterEach } from 'vitest'
+import { renderContent } from '#src/content-render/index.js'
+import { TitleFromAutotitleError } from '#src/content-render/unified/rewrite-local-links.js'
+
+describe('link error line numbers', () => {
+  let fs
+  let originalReadFileSync
+  let originalExistsSync
+  let mockContext
+
+  beforeEach(async () => {
+    // Set up file system mocking
+    fs = await import('fs')
+    originalReadFileSync = fs.default.readFileSync
+    originalExistsSync = fs.default.existsSync
+
+    fs.default.existsSync = () => true
+
+    // Set up basic mock context
+    mockContext = {
+      currentLanguage: 'en',
+      currentVersion: 'free-pro-team@latest',
+      pages: new Map(),
+      redirects: new Map(),
+      page: {
+        fullPath: '/fake/test-file.md',
+      },
+    }
+  })
+
+  afterEach(() => {
+    // Restore original functions
+    fs.default.readFileSync = originalReadFileSync
+    fs.default.existsSync = originalExistsSync
+  })
+
+  test('reports correct line numbers for broken AUTOTITLE links', async () => {
+    // Test content with frontmatter followed by content with a broken link
+    const template = `---
+title: Test Page
+version: 1.0
+---
+
+# Introduction
+
+This is some content.
+
+Here is a broken link: [AUTOTITLE](/nonexistent/page).
+
+More content here.`
+
+    fs.default.readFileSync = () => template
+
+    try {
+      await renderContent(template, mockContext)
+      expect.fail('Expected TitleFromAutotitleError to be thrown')
+    } catch (error) {
+      expect(error).toBeInstanceOf(TitleFromAutotitleError)
+
+      // The broken link is on line 10 in the original file
+      // (3 lines of frontmatter + 1 blank line + 1 title + 1 blank + 1 content + 1 blank + 1 link line)
+      // The error message should reference the correct line number
+      expect(error.message).toContain('/nonexistent/page')
+      expect(error.message).toContain('could not be resolved')
+      expect(error.message).toContain('(Line: 10)')
+    }
+  })
+
+  test('reports correct line numbers with different frontmatter sizes', async () => {
+    mockContext.page.fullPath = '/fake/test-file-2.md'
+
+    // Test with more extensive frontmatter
+    const template = `---
+title: Another Test Page
+description: This is a test
+author: Test Author
+date: 2024-01-01
+tags:
+  - test
+  - documentation
+version: 2.0
+---
+
+# Main Title
+
+Some introductory text here.
+
+## Section
+
+Content with a [AUTOTITLE](/another/nonexistent/page) link.`
+
+    fs.default.readFileSync = () => template
+
+    try {
+      await renderContent(template, mockContext)
+      expect.fail('Expected TitleFromAutotitleError to be thrown')
+    } catch (error) {
+      expect(error).toBeInstanceOf(TitleFromAutotitleError)
+      expect(error.message).toContain('/another/nonexistent/page')
+      expect(error.message).toContain('could not be resolved')
+    }
+  })
+
+  test('handles files without frontmatter correctly', async () => {
+    mockContext.page.fullPath = '/fake/no-frontmatter.md'
+
+    // Test content without frontmatter
+    const template = `# Simple Title
+
+This is content without frontmatter.
+
+Here is a broken link: [AUTOTITLE](/missing/page).`
+
+    fs.default.readFileSync = () => template
+
+    try {
+      await renderContent(template, mockContext)
+      expect.fail('Expected TitleFromAutotitleError to be thrown')
+    } catch (error) {
+      expect(error).toBeInstanceOf(TitleFromAutotitleError)
+      expect(error.message).toContain('/missing/page')
+      expect(error.message).toContain('could not be resolved')
+    }
+  })
+
+  test('error message format is improved', async () => {
+    mockContext.page.fullPath = '/fake/message-test.md'
+
+    const template = `---
+title: Message Test
+---
+
+[AUTOTITLE](/test/broken/link)
+`
+
+    fs.default.readFileSync = () => template
+
+    try {
+      await renderContent(template, mockContext)
+      expect.fail('Expected TitleFromAutotitleError to be thrown')
+    } catch (error) {
+      expect(error).toBeInstanceOf(TitleFromAutotitleError)
+
+      // Check that the new error message format is used
+      expect(error.message).toContain('could not be resolved in one or more versions')
+      expect(error.message).toContain('Make sure that this link can be reached from all versions')
+      expect(error.message).toContain('/test/broken/link')
+
+      // Check that the old error message format is NOT used
+      expect(error.message).not.toContain('Unable to find Page by')
+      expect(error.message).not.toContain('To fix it, look at')
+    }
+  })
+})

src/content-render/unified/rewrite-local-links.js

@@ -1,5 +1,4 @@
 import path from 'path'
-import fs from 'fs'
 
 import stripAnsi from 'strip-ansi'
 import { visit } from 'unist-util-visit'
@@ -46,36 +45,6 @@ function logError(file, line, message, title = 'Error') {
   }
 }
 
-function getFrontmatterOffset(filePath) {
-  if (!fs.existsSync(filePath)) return 0
-  const rawContent = fs.readFileSync(filePath, 'utf-8')
-  let delimiters = 0
-  let count = 0
-  // The frontmatter is wedged between two `---` lines. But the content
-  // doesn't necessarily start after the second `---`. If the `.md` file looks
-  // like this:
-  //
-  //     1) ---
-  //     2) title: Foo
-  //     3) ---
-  //     4)
-  //     5) # Introduction
-  //     6) Bla bla
-  //
-  // Then line one of the *content* that is processed, starts at line 5.
-  // because after the matter and content is separated, the content portion
-  // is whitespace trimmed.
-  for (const line of rawContent.split(/\n/g)) {
-    count++
-    if (line === '---') {
-      delimiters++
-    } else if (delimiters === 2 && line) {
-      return count
-    }
-  }
-  return 0
-}
-
 // Meaning it can be 'AUTOTITLE ' or ' AUTOTITLE' or 'AUTOTITLE'
 const AUTOTITLE = /^\s*AUTOTITLE\s*$/
 
@@ -204,12 +173,13 @@ async function getNewTitleSetter(child, href, context, originalHref) {
 async function getNewTitle(href, context, child, originalHref) {
   const page = findPage(href, context.pages, context.redirects)
   if (!page) {
-    const line = child.position.start.line + getFrontmatterOffset(context.page.fullPath)
-    const message = `Unable to find Page by '${originalHref || href}'.
-    To fix it, look at ${
-      context.page.fullPath
-    } on line ${line} and see if the link is correct and active.`
-    logError(context.page.fullPath, line, message)
+    // The child.position.start.line is 1-based and already represents the line number
+    // in the original file (including frontmatter), so no offset adjustment is needed
+    const line = child.position.start.line
+
+    const linkText = originalHref || href
+    const message = `The link '${linkText}' could not be resolved in one or more versions of the documentation. Make sure that this link can be reached from all versions of the documentation it appears in. (Line: ${line})`
+    logError(context.page.fullPath, line, message, 'Link Resolution Error')
     throw new TitleFromAutotitleError(message)
   }
   return await page.renderProp('title', context, { textOnly: true })