Skip to content

Commit

Permalink
Merge pull request github#28491 from github/repo-sync
Browse files Browse the repository at this point in the history
Repo sync
  • Loading branch information
docs-bot authored Sep 25, 2023
2 parents 7555c8a + b8dfb50 commit d888bfa
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 73 deletions.
2 changes: 2 additions & 0 deletions content/contributing/writing-for-github-docs/style-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ Avoid inline links in command names.
When code examples refer to a larger file, show the relevant section of the file, so that users understand how to edit their own code in context.
- **Use:**

<!-- markdownlint-disable yaml-scheduled-jobs -->
```yaml
on:
schedule:
Expand All @@ -138,6 +139,7 @@ on:
schedule:
- cron: "40 19 * * *"
```
<!-- markdownlint-enable yaml-scheduled-jobs -->
### File names and directory names
Expand Down
4 changes: 2 additions & 2 deletions src/content-linter/lib/helpers/get-rules.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import rules from '../../../../node_modules/markdownlint/lib/rules.js'
import { gitHubDocsMarkdownlint } from '../linting-rules/index.js'
import { baseConfig } from '../../style/base.js'
import { githubDocsConfig, searchReplaceConfig } from '../../style/github-docs.js'
import { customConfig } from '../../style/github-docs.js'

export const customRules = gitHubDocsMarkdownlint.rules
export const allRules = [...rules, ...gitHubDocsMarkdownlint.rules]
export const allConfig = { ...baseConfig, ...githubDocsConfig, ...searchReplaceConfig }
export const allConfig = { ...baseConfig, ...customConfig }
9 changes: 9 additions & 0 deletions src/content-linter/lib/helpers/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ export function isStringPunctuated(text) {
return /^.*[.?!]['"]?$/.test(text)
}

export function doesStringEndWithPeriod(text) {
// String ends with punctuation of either
// . ? ! and optionally ends with single
// or double quotes. This also allows
// for single or double quotes before
// the punctuation.
return /^.*\.['"]?$/.test(text)
}

// Filters a list of tokens by token type only when they match
// a specific token type order.
// For example, if a list of tokens contains:
Expand Down
4 changes: 2 additions & 2 deletions src/content-linter/lib/linting-rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { internalLinksLang } from './internal-links-lang.js'
import { internalLinksSlash } from './internal-links-slash.js'
import { imageAltTextExcludeStartWords } from './image-alt-text-exclude-start-words.js'
import { listFirstWordCapitalization } from './list-first-word-capitalization.js'
import { internalLinkPunctuation } from './internal-link-punctuation.js'
import { linkPunctuation } from './link-punctuation.js'
import { earlyAccessReferences } from './early-access-references.js'
import { yamlScheduledJobs } from './yaml-scheduled-jobs.js'

Expand All @@ -23,7 +23,7 @@ export const gitHubDocsMarkdownlint = {
internalLinksSlash,
imageAltTextExcludeStartWords,
listFirstWordCapitalization,
internalLinkPunctuation,
linkPunctuation,
earlyAccessReferences,
yamlScheduledJobs,
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { filterTokens, addError } from 'markdownlint-rule-helpers'

import { getRange, isStringQuoted, isStringPunctuated } from '../helpers/utils.js'
import { getRange, isStringQuoted, doesStringEndWithPeriod } from '../helpers/utils.js'

export const internalLinkPunctuation = {
names: ['GHD008', 'internal-link-punctuation'],
export const linkPunctuation = {
names: ['GHD008', 'link-punctuation'],
description: 'Internal link titles must not contain punctuation',
tags: ['links', 'url'],
information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'),
Expand All @@ -18,15 +18,15 @@ export const internalLinkPunctuation = {
inLink = false
} else if (inLink && child.type === 'text') {
const content = child.content.trim()
const hasPuntuation = isStringPunctuated(content)
const hasPeriod = doesStringEndWithPeriod(content)
const hasQuotes = isStringQuoted(content)

if (hasPuntuation || hasQuotes) {
if (hasPeriod || hasQuotes) {
const range = getRange(line, content)
addError(
onError,
child.lineNumber,
'Remove ", \', ?, !, and . characters from the link title.',
'Remove quotes and/or period punctuation from the link title.',
child.content,
range,
null, // No fix possible
Expand Down
7 changes: 4 additions & 3 deletions src/content-linter/scripts/lint-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { execSync } from 'child_process'

import walkFiles from '../../../script/helpers/walk-files.js'
import { allConfig, allRules, customRules } from '../lib/helpers/get-rules.js'
import { githubDocsConfig } from '../style/github-docs.js'
import { customConfig } from '../style/github-docs.js'

program
.description('Run GitHub Docs Markdownlint rules.')
Expand Down Expand Up @@ -308,7 +308,7 @@ function listRules() {
Rules that can't be run on partials have the property
`partial-markdown-files` set to false.
*/
function getMarkdownLintConfig(errorsOnly, runRules, customRules) {
function getMarkdownLintConfig(errorsOnly, runRules) {
const config = {
content: {
default: false, // By default, don't turn on all markdownlint rules
Expand All @@ -323,12 +323,13 @@ function getMarkdownLintConfig(errorsOnly, runRules, customRules) {
}

for (const [ruleName, ruleConfig] of Object.entries(allConfig)) {
const customRule = githubDocsConfig[ruleName] && getCustomRule(ruleName)
const customRule = customConfig[ruleName] && getCustomRule(ruleName)

// search-replace is handled differently than other rules because
// it has nested metadata and rules.
if (errorsOnly && getRuleSeverity(ruleConfig) !== 'error' && ruleName !== 'search-replace')
continue

if (runRules && !runRules.includes(ruleName)) continue

// Handle the special case of the search-replace rule
Expand Down
14 changes: 12 additions & 2 deletions src/content-linter/style/github-docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const githubDocsConfig = {
severity: 'error',
'partial-markdown-files': true,
},
'internal-link-punctuation': {
'link-punctuation': {
// GHD008
severity: 'warning',
'partial-markdown-files': true,
Expand Down Expand Up @@ -72,7 +72,7 @@ export const searchReplaceConfig = {
message: 'Catch occurrences of docs.gitub.com domain.',
search: 'docs.github.com',
searchScope: 'all',
severity: 'error',
severity: 'warning',
'partial-markdown-files': true,
},
{
Expand Down Expand Up @@ -110,6 +110,8 @@ export const searchReplaceConfig = {
message: 'Catch occurrences of deprecated liquid data syntax.',
searchPattern: '/{{\\s*?site\\.data\\.([a-zA-Z0-9-_]+(?:\\.[a-zA-Z0-9-_]+)+)\\s*?}}/g',
replace: '{% data $1 %}',
severity: 'error',
'partial-markdown-files': true,
},
{
// Catches usage of old octicon variable syntax. For example:
Expand All @@ -119,6 +121,8 @@ export const searchReplaceConfig = {
message:
'The octicon liquid syntax used is deprecated. Use this format instead {% octicon "<octicon-name>" aria-label="<Octicon aria label>" %}',
searchPattern: '/{{\\s*?octicon-([a-z-]+)(\\s[\\w\\s\\d-]+)?\\s*?}}/g',
severity: 'error',
'partial-markdown-files': true,
},
{
// Catches usage of string personal access token, which should
Expand All @@ -127,6 +131,8 @@ export const searchReplaceConfig = {
message:
'The string "personal access token" can be replaced with a variable. You should use one of the variables from data/variables/product.yml instead of the literal phrase(s):',
searchPattern: '/personal access tokens?/gi',
severity: 'warning',
'partial-markdown-files': true,
},
{
// Catches usage of GitHub-owned actions that don't use a
Expand Down Expand Up @@ -156,7 +162,11 @@ export const searchReplaceConfig = {
'A GitHub-owned action is referenced, but should be replaced with a reusable from data/reusables/actions.',
searchPattern:
'/(actions\\/(checkout|delete-package-versions|download-artifact|upload-artifact|github-script|setup-dotnet|setup-go|setup-java|setup-node|setup-python|stale|cache)|github\\/codeql-action[/a-zA-Z-]*)/g',
severity: 'warning',
'partial-markdown-files': true,
},
],
},
}

export const customConfig = { ...searchReplaceConfig, ...githubDocsConfig }
57 changes: 0 additions & 57 deletions src/content-linter/tests/unit/internal-link-punctuation.js

This file was deleted.

57 changes: 57 additions & 0 deletions src/content-linter/tests/unit/link-punctuation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { jest } from '@jest/globals'

import { runRule } from '../../lib/init-test.js'
import { linkPunctuation } from '../../lib/linting-rules/link-punctuation.js'

jest.setTimeout(60 * 1000)

describe(linkPunctuation.names.join(' - '), () => {
test('inline links without quotes or a period should not error', async () => {
const markdown = [
'[This should pass](./image.png)',
'[AUTOTITLE](./image.png)',
// These are not necessarily good descriptions, but they are valid
// per the requirements of the rule
"[A link with end quote'](./image.png)",
'["A link with start quote](./image.png)',
'[A link with a question mark?](./image.png)',
'[A link with an exclamation point!](./image.png)',
].join('\n')

const result = await runRule(linkPunctuation, { strings: { markdown } })
const errors = result.markdown
expect(errors.length).toBe(0)
})
test('inline links with quotes or punctuation should error', async () => {
const markdown = [
'["A title"](./image.png)',
"['A title'](./image.png)",
'[A title.](./image.png)',
'["A title."](./image.png)',
'["A title".](./image.png)',
"['A title.'](./image.png)",
"['A title'.](./image.png)",
"['A title'?](./image.png)",
'["A title"!](./image.png)',
"['A title?'](./image.png)",
'["A title!"](./image.png)',
].join('\n')

const result = await runRule(linkPunctuation, { strings: { markdown } })
const errors = result.markdown
expect(errors.length).toBe(11)
expect(errors[0].errorRange).toEqual([2, 9])
expect(errors[6].lineNumber).toBe(7)
})
test('links that is not plain text', async () => {
const markdown = [
'[*emphasize*](./image.png)',
'[**boldness**](./image.png)',
'[**boldness** and *emphasize*](./image.png)',
].join('\n')

const result = await runRule(linkPunctuation, { strings: { markdown } })
const errors = result.markdown
expect(errors.length).toBe(0)
})
})
3 changes: 2 additions & 1 deletion src/content-linter/tests/unit/search-replace.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ describe(searchReplace.names.join(' - '), () => {
'developer.github.com/changes/',
'developer.github.com/changes/changes',
'developer.github.com/enterprise/1',
'<https://docs.github.com/en/rest/reference/code-scanning#upload-an-analysis-as-sarif-data>',
].join('\n')
const result = await runRule(searchReplace, {
strings: { markdown },
testConfig: searchReplaceConfig['search-replace'],
})
const errors = result.markdown
expect(errors.length).toBe(9)
expect(errors.length).toBe(10)
})

test('Deprecated Liquid syntax causes error', async () => {
Expand Down

0 comments on commit d888bfa

Please sign in to comment.