Skip to content

Commit 0d6515c

Browse files
authored
Merge pull request #39223 from github/repo-sync
Repo sync
2 parents 311b7f3 + 0ce6e5e commit 0d6515c

File tree

7 files changed

+324
-211
lines changed

7 files changed

+324
-211
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ If the primary goal in showing a dropdown menu is to help the reader distinguish
130130

131131
## Highlighting elements in screenshots
132132

133-
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.
133+
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.
134134

135135
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/).
136136

data/reusables/advanced-security/delegated-alert-dismissal-beta.md

Lines changed: 0 additions & 6 deletions
This file was deleted.

data/reusables/gated-features/security-features-basic.md

Lines changed: 0 additions & 5 deletions
This file was deleted.
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import { describe, expect, test, beforeEach, afterEach } from 'vitest'
2+
import { renderContent } from '#src/content-render/index.js'
3+
import { TitleFromAutotitleError } from '#src/content-render/unified/rewrite-local-links.js'
4+
5+
describe('link error line numbers', () => {
6+
let fs
7+
let originalReadFileSync
8+
let originalExistsSync
9+
let mockContext
10+
11+
beforeEach(async () => {
12+
// Set up file system mocking
13+
fs = await import('fs')
14+
originalReadFileSync = fs.default.readFileSync
15+
originalExistsSync = fs.default.existsSync
16+
17+
fs.default.existsSync = () => true
18+
19+
// Set up basic mock context
20+
mockContext = {
21+
currentLanguage: 'en',
22+
currentVersion: 'free-pro-team@latest',
23+
pages: new Map(),
24+
redirects: new Map(),
25+
page: {
26+
fullPath: '/fake/test-file.md',
27+
},
28+
}
29+
})
30+
31+
afterEach(() => {
32+
// Restore original functions
33+
fs.default.readFileSync = originalReadFileSync
34+
fs.default.existsSync = originalExistsSync
35+
})
36+
37+
test('reports correct line numbers for broken AUTOTITLE links', async () => {
38+
// Test content with frontmatter followed by content with a broken link
39+
const template = `---
40+
title: Test Page
41+
version: 1.0
42+
---
43+
44+
# Introduction
45+
46+
This is some content.
47+
48+
Here is a broken link: [AUTOTITLE](/nonexistent/page).
49+
50+
More content here.`
51+
52+
fs.default.readFileSync = () => template
53+
54+
try {
55+
await renderContent(template, mockContext)
56+
expect.fail('Expected TitleFromAutotitleError to be thrown')
57+
} catch (error) {
58+
expect(error).toBeInstanceOf(TitleFromAutotitleError)
59+
60+
// The broken link is on line 10 in the original file
61+
// (3 lines of frontmatter + 1 blank line + 1 title + 1 blank + 1 content + 1 blank + 1 link line)
62+
// The error message should reference the correct line number
63+
expect(error.message).toContain('/nonexistent/page')
64+
expect(error.message).toContain('could not be resolved')
65+
expect(error.message).toContain('(Line: 10)')
66+
}
67+
})
68+
69+
test('reports correct line numbers with different frontmatter sizes', async () => {
70+
mockContext.page.fullPath = '/fake/test-file-2.md'
71+
72+
// Test with more extensive frontmatter
73+
const template = `---
74+
title: Another Test Page
75+
description: This is a test
76+
author: Test Author
77+
date: 2024-01-01
78+
tags:
79+
- test
80+
- documentation
81+
version: 2.0
82+
---
83+
84+
# Main Title
85+
86+
Some introductory text here.
87+
88+
## Section
89+
90+
Content with a [AUTOTITLE](/another/nonexistent/page) link.`
91+
92+
fs.default.readFileSync = () => template
93+
94+
try {
95+
await renderContent(template, mockContext)
96+
expect.fail('Expected TitleFromAutotitleError to be thrown')
97+
} catch (error) {
98+
expect(error).toBeInstanceOf(TitleFromAutotitleError)
99+
expect(error.message).toContain('/another/nonexistent/page')
100+
expect(error.message).toContain('could not be resolved')
101+
}
102+
})
103+
104+
test('handles files without frontmatter correctly', async () => {
105+
mockContext.page.fullPath = '/fake/no-frontmatter.md'
106+
107+
// Test content without frontmatter
108+
const template = `# Simple Title
109+
110+
This is content without frontmatter.
111+
112+
Here is a broken link: [AUTOTITLE](/missing/page).`
113+
114+
fs.default.readFileSync = () => template
115+
116+
try {
117+
await renderContent(template, mockContext)
118+
expect.fail('Expected TitleFromAutotitleError to be thrown')
119+
} catch (error) {
120+
expect(error).toBeInstanceOf(TitleFromAutotitleError)
121+
expect(error.message).toContain('/missing/page')
122+
expect(error.message).toContain('could not be resolved')
123+
}
124+
})
125+
126+
test('error message format is improved', async () => {
127+
mockContext.page.fullPath = '/fake/message-test.md'
128+
129+
const template = `---
130+
title: Message Test
131+
---
132+
133+
[AUTOTITLE](/test/broken/link)
134+
`
135+
136+
fs.default.readFileSync = () => template
137+
138+
try {
139+
await renderContent(template, mockContext)
140+
expect.fail('Expected TitleFromAutotitleError to be thrown')
141+
} catch (error) {
142+
expect(error).toBeInstanceOf(TitleFromAutotitleError)
143+
144+
// Check that the new error message format is used
145+
expect(error.message).toContain('could not be resolved in one or more versions')
146+
expect(error.message).toContain('Make sure that this link can be reached from all versions')
147+
expect(error.message).toContain('/test/broken/link')
148+
149+
// Check that the old error message format is NOT used
150+
expect(error.message).not.toContain('Unable to find Page by')
151+
expect(error.message).not.toContain('To fix it, look at')
152+
}
153+
})
154+
})

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

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import path from 'path'
2-
import fs from 'fs'
32

43
import stripAnsi from 'strip-ansi'
54
import { visit } from 'unist-util-visit'
@@ -46,36 +45,6 @@ function logError(file, line, message, title = 'Error') {
4645
}
4746
}
4847

49-
function getFrontmatterOffset(filePath) {
50-
if (!fs.existsSync(filePath)) return 0
51-
const rawContent = fs.readFileSync(filePath, 'utf-8')
52-
let delimiters = 0
53-
let count = 0
54-
// The frontmatter is wedged between two `---` lines. But the content
55-
// doesn't necessarily start after the second `---`. If the `.md` file looks
56-
// like this:
57-
//
58-
// 1) ---
59-
// 2) title: Foo
60-
// 3) ---
61-
// 4)
62-
// 5) # Introduction
63-
// 6) Bla bla
64-
//
65-
// Then line one of the *content* that is processed, starts at line 5.
66-
// because after the matter and content is separated, the content portion
67-
// is whitespace trimmed.
68-
for (const line of rawContent.split(/\n/g)) {
69-
count++
70-
if (line === '---') {
71-
delimiters++
72-
} else if (delimiters === 2 && line) {
73-
return count
74-
}
75-
}
76-
return 0
77-
}
78-
7948
// Meaning it can be 'AUTOTITLE ' or ' AUTOTITLE' or 'AUTOTITLE'
8049
const AUTOTITLE = /^\s*AUTOTITLE\s*$/
8150

@@ -204,12 +173,13 @@ async function getNewTitleSetter(child, href, context, originalHref) {
204173
async function getNewTitle(href, context, child, originalHref) {
205174
const page = findPage(href, context.pages, context.redirects)
206175
if (!page) {
207-
const line = child.position.start.line + getFrontmatterOffset(context.page.fullPath)
208-
const message = `Unable to find Page by '${originalHref || href}'.
209-
To fix it, look at ${
210-
context.page.fullPath
211-
} on line ${line} and see if the link is correct and active.`
212-
logError(context.page.fullPath, line, message)
176+
// The child.position.start.line is 1-based and already represents the line number
177+
// in the original file (including frontmatter), so no offset adjustment is needed
178+
const line = child.position.start.line
179+
180+
const linkText = originalHref || href
181+
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})`
182+
logError(context.page.fullPath, line, message, 'Link Resolution Error')
213183
throw new TitleFromAutotitleError(message)
214184
}
215185
return await page.renderProp('title', context, { textOnly: true })

0 commit comments

Comments
 (0)