Skip to content

Commit 8370a2b

Browse files
authored
Markdownlint script output formatting and error updates (github#38953)
1 parent 3bc1f7c commit 8370a2b

File tree

9 files changed

+99
-57
lines changed

9 files changed

+99
-57
lines changed

src/content-linter/lib/helpers.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { addError, newLineRe } from 'markdownlint-rule-helpers'
2+
3+
// Adds an error object with details conditionally via the onError callback
4+
export function addFixErrorDetail(onError, lineNumber, expected, actual, range, fixInfo) {
5+
addError(onError, lineNumber, `Expected: ${expected}`, ` Actual: ${actual}`, range, fixInfo)
6+
}
7+
8+
export function getCodeFenceTokens(params) {
9+
return params.tokens.filter((t) => t.type === 'fence')
10+
}
11+
12+
export function getCodeFenceLines(token) {
13+
return token.content.split(newLineRe)
14+
}

src/content-linter/lib/linting-rules/code-fence-line-length.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import { addError } from 'markdownlint-rule-helpers'
1+
import { addError, ellipsify } from 'markdownlint-rule-helpers'
22

3-
import { getCodeFenceTokens, getCodeFenceLines } from '../markdownlint-helpers.js'
3+
import { getCodeFenceTokens, getCodeFenceLines } from '../helpers.js'
44

55
export const codeFenceLineLength = {
66
names: ['GHD001', 'code-fence-line-length'],
77
description: 'Code fence lines should not exceed a maximum length',
88
tags: ['code'],
99
severity: 'warning',
10+
information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'),
1011
function: function GHD001(params, onError) {
1112
const MAX_LINE_LENGTH = String(params.config.maxLength || 60)
1213
const codeFenceTokens = getCodeFenceTokens(params)
@@ -22,8 +23,9 @@ export const codeFenceLineLength = {
2223
onError,
2324
lineNumber,
2425
`Code fence line exceeds ${MAX_LINE_LENGTH} characters.`,
25-
undefined, // N/A
26-
undefined, // N/A
26+
ellipsify(line),
27+
[1, line.length],
28+
null, // No fix possible
2729
)
2830
}
2931
})

src/content-linter/lib/linting-rules/image-alt-text-end-punctuation.js

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,30 @@
1-
import { addError, forEachInlineChild } from 'markdownlint-rule-helpers'
1+
import { forEachInlineChild } from 'markdownlint-rule-helpers'
2+
3+
import { addFixErrorDetail } from '../helpers.js'
24

35
export const imageAltTextEndPunctuation = {
46
names: ['GHD002', 'image-alt-text-end-punctuation'],
5-
description: 'Images alternate text should end with a punctuation.',
7+
description: 'Alternate text for images should end with a punctuation.',
68
severity: 'error',
79
tags: ['accessibility', 'images'],
10+
information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'),
811
function: function GHD003(params, onError) {
912
forEachInlineChild(params, 'image', function forToken(token) {
1013
const quoteRegex = /[.?!]['"]$/
1114
const endRegex = /[.?!]$/
1215
const imageAltText = token.content.trim()
16+
const startColumnIndex = token.line.indexOf(imageAltText)
17+
const range = startColumnIndex ? [startColumnIndex + 1, imageAltText.length] : null
1318
if (
1419
(!imageAltText.endsWith('"') && !imageAltText.slice(-1).match(endRegex)) ||
1520
(imageAltText.endsWith('"') && !imageAltText.slice(-2).match(quoteRegex))
1621
) {
17-
addError(
18-
onError,
19-
token.lineNumber,
20-
`On line ${token.lineNumber}, the image alt text '${imageAltText}' must have punctuation at the end of the sentence.`,
21-
undefined,
22-
undefined,
23-
{
24-
lineNumber: token.lineNumber,
25-
editColumn: token.line.indexOf(']') + 1,
26-
deleteCount: 0,
27-
insertText: '.',
28-
},
29-
)
22+
addFixErrorDetail(onError, token.lineNumber, imageAltText + '.', imageAltText, range, {
23+
lineNumber: token.lineNumber,
24+
editColumn: token.line.indexOf(']') + 1,
25+
deleteCount: 0,
26+
insertText: '.',
27+
})
3028
}
3129
})
3230
},

src/content-linter/lib/linting-rules/image-alt-text-length.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export const incorrectAltTextLength = {
66
severity: 'warning',
77
description: 'Images alternate text should be between 40-150 characters',
88
tags: ['accessibility', 'images'],
9+
information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'),
910
function: function GHD004(params, onError) {
1011
forEachInlineChild(params, 'image', async function forToken(token) {
1112
let renderedString = token.content
@@ -18,7 +19,10 @@ export const incorrectAltTextLength = {
1819
addError(
1920
onError,
2021
token.lineNumber,
21-
`The alt text: ${renderedString}, is ${renderedString.length} characters long`,
22+
`Image alternate text is ${renderedString.length} characters long.`,
23+
renderedString,
24+
null, // No range
25+
null, // No fix possible
2226
)
2327
}
2428
})

src/content-linter/lib/linting-rules/image-file-kebab.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,26 @@
1-
import { addError, forEachInlineChild } from 'markdownlint-rule-helpers'
1+
import { forEachInlineChild } from 'markdownlint-rule-helpers'
2+
3+
import { addFixErrorDetail } from '../helpers.js'
24

35
export const imageFileKebab = {
46
names: ['GHD004', 'image-file-kebab'],
57
description: 'Image file names should always be lowercase kebab case',
68
severity: 'warning',
79
tags: ['accessibility', 'images'],
10+
information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'),
811
function: function GHD005(params, onError) {
912
forEachInlineChild(params, 'image', async function forToken(token) {
1013
const imageFileName = token.attrs[0][1].split('/').pop().split('.')[0]
1114
const nonKebabRegex = /([A-Z]|_)/
15+
const suggestedFileName = imageFileName.toLowerCase().replace(/_/g, '-')
1216
if (imageFileName.match(nonKebabRegex)) {
13-
addError(
17+
addFixErrorDetail(
1418
onError,
1519
token.lineNumber,
16-
`The image file name: ${token.attrs[0][1]}, is not lowercase kebab case.`,
20+
imageFileName,
21+
suggestedFileName,
22+
[token.line.indexOf(imageFileName) + 1, imageFileName.length],
23+
null, // Todo add fix
1724
)
1825
}
1926
})

src/content-linter/lib/linting-rules/internal-links-lang.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
1-
import { addError, filterTokens } from 'markdownlint-rule-helpers'
1+
import { filterTokens } from 'markdownlint-rule-helpers'
22

3+
import { addFixErrorDetail } from '../helpers.js'
34
import { languageKeys } from '../../../../lib/languages.js'
45

56
export const internalLinksLang = {
67
names: ['GHD005', 'internal-links-lang'],
78
description: 'Internal links must not have a hardcoded language code',
89
severity: 'error',
910
tags: ['links', 'url'],
11+
information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'),
1012
function: function GHD006(params, onError) {
1113
filterTokens(params, 'inline', (token) => {
1214
let linkHref = ''
@@ -16,21 +18,22 @@ export const internalLinksLang = {
1618
linkHref = ''
1719
for (const attr of child.attrs) {
1820
if (
19-
languageKeys.includes(attr[1].split('/')[1]) ||
20-
languageKeys.includes(attr[1].split('/')[0])
21+
languageKeys.some(
22+
(lang) => attr[1].startsWith(`/${lang}`) || attr[1].startsWith(lang),
23+
)
2124
) {
2225
internalLinkHasLang = true
2326
linkHref = attr[1]
2427
}
2528
}
2629
} else if (child.type === 'link_close') {
2730
if (internalLinkHasLang) {
28-
addError(
31+
addFixErrorDetail(
2932
onError,
3033
child.lineNumber,
31-
`This internal link: ${linkHref} must not start with a hardcoded language code.`,
32-
undefined,
33-
undefined,
34+
linkHref.replace(/(\/)?[a-z]{2}/, ''),
35+
linkHref,
36+
undefined, // Todo add range
3437
{
3538
lineNumber: child.lineNumber,
3639
editColumn: token.line.indexOf('(') + 2,

src/content-linter/lib/linting-rules/internal-links-slash.js

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1-
import { addError, filterTokens } from 'markdownlint-rule-helpers'
1+
import { filterTokens } from 'markdownlint-rule-helpers'
2+
3+
import { addFixErrorDetail } from '../helpers.js'
24

35
export const internalLinksSlash = {
46
names: ['GHD006', 'internal-links-slash'],
57
description: 'Internal links must start with a /',
68
severity: 'error',
79
tags: ['links', 'url'],
10+
information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'),
811
function: function GHD007(params, onError) {
912
filterTokens(params, 'inline', (token) => {
1013
let linkHref = ''
@@ -26,19 +29,12 @@ export const internalLinksSlash = {
2629
}
2730
} else if (child.type === 'link_close') {
2831
if (!internalLinkHasSlash) {
29-
addError(
30-
onError,
31-
child.lineNumber,
32-
`This relative link: ${linkHref} on line ${child.lineNumber} must start with a /`,
33-
undefined,
34-
undefined,
35-
{
36-
lineNumber: child.lineNumber,
37-
editColumn: token.line.indexOf('(') + 2,
38-
deleteCount: 0,
39-
insertText: '/',
40-
},
41-
)
32+
addFixErrorDetail(onError, child.lineNumber, `/${linkHref}`, linkHref, undefined, {
33+
lineNumber: child.lineNumber,
34+
editColumn: token.line.indexOf('(') + 2,
35+
deleteCount: 0,
36+
insertText: '/',
37+
})
4238
internalLinkHasSlash = true
4339
}
4440
}

src/content-linter/lib/markdownlint-helpers.js

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

src/content-linter/scripts/markdownlint.js

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ program
2828
)
2929
.option('--fix', 'Fix linting errors.')
3030
.option('--summary-by-rule', 'Summarize the number of errors for each rule.')
31+
.option('--verbose', 'Output full format for all errors.')
3132
.parse(process.argv)
3233

33-
const { files, fix, paths, error, rules, summaryByRule } = program.opts()
34+
const { files, fix, paths, error, rules, summaryByRule, verbose } = program.opts()
3435
const DEFAULT_LINT_DIRS = ['content', 'data']
3536

3637
main()
@@ -77,7 +78,7 @@ async function main() {
7778
}
7879
}
7980

80-
// Used for a temporary way to allow us to see how many errors currently
81+
// Used for a temparary way to allow us to see how many errors currently
8182
// exist for each rule in the content directory.
8283
if (summaryByRule) {
8384
reportSummaryByRule(result, config)
@@ -129,12 +130,38 @@ function reportSummaryByRule(result, config) {
129130
function reportResults(results) {
130131
Object.entries(results)
131132
// each result key always has an array value, but it may be empty
132-
.filter(([key, result]) => result.length)
133+
.filter(([, result]) => result.length)
133134
.forEach(([key, result]) => {
134-
console.log(key, result)
135+
console.log(key)
136+
if (!verbose) {
137+
result.forEach((flaw) => {
138+
const simplifiedResult = formatResult(flaw)
139+
console.log(simplifiedResult)
140+
})
141+
} else {
142+
console.log(result)
143+
}
135144
})
136145
}
137146

138147
function getErrorCountByFile(result) {
139148
return Object.values(result).filter((value) => value.length).length
140149
}
150+
151+
function formatResult(object) {
152+
return Object.entries(object).reduce((acc, [key, value]) => {
153+
if (key === 'fixInfo') {
154+
acc.fixable = !!value
155+
delete acc.fixInfo
156+
return acc
157+
}
158+
if (!value) return acc
159+
if (key === 'errorRange') {
160+
acc.columnNumber = value[0]
161+
delete acc.range
162+
return acc
163+
}
164+
acc[key] = value
165+
return acc
166+
}, {})
167+
}

0 commit comments

Comments
 (0)