Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use correct header level when splitting rules list into sub-lists with --split-by #271

Merged
merged 1 commit into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 8 additions & 14 deletions lib/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,16 @@ export function replaceOrCreateHeader(
const markerLineIndex = lines.indexOf(marker);
const dashesLineIndex1 = lines.indexOf('---');
const dashesLineIndex2 = lines.indexOf('---', dashesLineIndex1 + 1);
const hasTitle = titleLineIndex !== -1;
const hasMarker = markerLineIndex !== -1;
const hasYamlFrontMatter = dashesLineIndex1 === 0 && dashesLineIndex2 !== -1;

// Any YAML front matter or anything else above the title should be kept as-is ahead of the new header.
const preHeader = lines
.slice(
0,
hasTitle ? titleLineIndex : hasYamlFrontMatter ? dashesLineIndex2 + 1 : 0
)
.slice(0, Math.max(titleLineIndex, dashesLineIndex2 + 1))
.join('\n');

// Anything after the marker comment, title, or YAML front matter should be kept as-is after the new header.
const postHeader = lines
.slice(
hasMarker
? markerLineIndex + 1
: hasTitle
? titleLineIndex + 1
: hasYamlFrontMatter
? dashesLineIndex2 + 1
: 0
Math.max(markerLineIndex + 1, titleLineIndex + 1, dashesLineIndex2 + 1)
)
.join('\n');

Expand Down Expand Up @@ -74,3 +62,9 @@ export function findSectionHeader(
(a: string, b: string) => a.length - b.length
)[0];
}

export function findFinalHeaderLevel(str: string) {
const lines = str.split('\n');
const finalHeader = lines.reverse().find((line) => line.match('^(#+) .+$'));
return finalHeader ? finalHeader.indexOf(' ') : undefined;
}
14 changes: 12 additions & 2 deletions lib/rule-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from './emojis.js';
import { getEmojisForConfigsSettingRuleToSeverity } from './plugin-configs.js';
import { getColumns, COLUMN_HEADER } from './rule-list-columns.js';
import { findSectionHeader } from './markdown.js';
import { findSectionHeader, findFinalHeaderLevel } from './markdown.js';
import { getPluginRoot } from './package-json.js';
import { generateLegend } from './rule-list-legend.js';
import { relative } from 'node:path';
Expand Down Expand Up @@ -215,6 +215,7 @@ function generateRulesListMarkdownWithSplitBy(
configEmojis: ConfigEmojis,
ignoreConfig: string[],
splitBy: string,
headerLevel: number,
urlRuleDoc?: string
): string {
const values = new Set(
Expand Down Expand Up @@ -276,7 +277,9 @@ function generateRulesListMarkdownWithSplitBy(
: splitByFinalPart;

parts.push(
`### ${ENABLED_VALUES.has(value) ? splitByTitle : value}`,
`${'#'.repeat(headerLevel)} ${
ENABLED_VALUES.has(value) ? splitByTitle : value
}`,
generateRulesListMarkdown(
columns,
rulesForThisValue,
Expand Down Expand Up @@ -344,6 +347,12 @@ export function updateRulesList(
const preList = markdown.slice(0, Math.max(0, listStartIndex));
const postList = markdown.slice(Math.max(0, listEndIndex));

// Determine what header level to use for sub-lists based on the last seen header level.
const preListFinalHeaderLevel = findFinalHeaderLevel(preList);
const splitByHeaderLevel = preListFinalHeaderLevel
? preListFinalHeaderLevel + 1
: 1;

// Determine columns to include in the rules list.
const columns = getColumns(
plugin,
Expand Down Expand Up @@ -377,6 +386,7 @@ export function updateRulesList(
configEmojis,
ignoreConfig,
splitBy,
splitByHeaderLevel,
urlRuleDoc
)
: generateRulesListMarkdown(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,28 @@ exports[`generate (--split-by) with boolean splits the list 1`] = `
"
`;

exports[`generate (--split-by) with no existing headers in file uses the proper sub-list header level 1`] = `
"<!-- begin auto-generated rules list -->

| Name |
| :----------------------------- |
| [no-baz](docs/rules/no-baz.md) |

# candy

| Name |
| :----------------------------- |
| [no-bar](docs/rules/no-bar.md) |

# fruits

| Name |
| :----------------------------- |
| [no-foo](docs/rules/no-foo.md) |

<!-- end auto-generated rules list -->"
`;

exports[`generate (--split-by) with one sub-list having no rules enabled by the config splits the list and still uses recommended config emoji in both lists 1`] = `
"## Rules
<!-- begin auto-generated rules list -->
Expand All @@ -119,6 +141,29 @@ exports[`generate (--split-by) with one sub-list having no rules enabled by the
"
`;

exports[`generate (--split-by) with only a title in the rules file uses the proper sub-list header level 1`] = `
"# Rules
<!-- begin auto-generated rules list -->

| Name |
| :----------------------------- |
| [no-baz](docs/rules/no-baz.md) |

## candy

| Name |
| :----------------------------- |
| [no-bar](docs/rules/no-bar.md) |

## fruits

| Name |
| :----------------------------- |
| [no-foo](docs/rules/no-foo.md) |

<!-- end auto-generated rules list -->"
`;

exports[`generate (--split-by) with unknown variable type splits the list but does not attempt to convert variable name to title 1`] = `
"## Rules
<!-- begin auto-generated rules list -->
Expand Down
82 changes: 82 additions & 0 deletions test/lib/generate/option-rule-list-split-by-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,88 @@ describe('generate (--split-by)', function () {
});
});

describe('with no existing headers in file', function () {
beforeEach(function () {
mockFs({
'package.json': JSON.stringify({
name: 'eslint-plugin-test',
exports: 'index.js',
type: 'module',
}),

'index.js': `
export default {
rules: {
'no-foo': { meta: { docs: { category: 'fruits' } }, create(context) {} },
'no-bar': { meta: { docs: { category: 'candy' } }, create(context) {} },
'no-baz': { meta: { /* no nested object */ }, create(context) {} },
},
};`,

'README.md':
'<!-- begin auto-generated rules list --><!-- end auto-generated rules list -->',

'docs/rules/no-foo.md': '',
'docs/rules/no-bar.md': '',
'docs/rules/no-baz.md': '',

// Needed for some of the test infrastructure to work.
node_modules: mockFs.load(PATH_NODE_MODULES),
});
});

afterEach(function () {
mockFs.restore();
jest.resetModules();
});

it('uses the proper sub-list header level', async function () {
await generate('.', { splitBy: 'meta.docs.category' });
expect(readFileSync('README.md', 'utf8')).toMatchSnapshot();
});
});

describe('with only a title in the rules file', function () {
beforeEach(function () {
mockFs({
'package.json': JSON.stringify({
name: 'eslint-plugin-test',
exports: 'index.js',
type: 'module',
}),

'index.js': `
export default {
rules: {
'no-foo': { meta: { docs: { category: 'fruits' } }, create(context) {} },
'no-bar': { meta: { docs: { category: 'candy' } }, create(context) {} },
'no-baz': { meta: { /* no nested object */ }, create(context) {} },
},
};`,

'README.md':
'# Rules\n<!-- begin auto-generated rules list --><!-- end auto-generated rules list -->',

'docs/rules/no-foo.md': '',
'docs/rules/no-bar.md': '',
'docs/rules/no-baz.md': '',

// Needed for some of the test infrastructure to work.
node_modules: mockFs.load(PATH_NODE_MODULES),
});
});

afterEach(function () {
mockFs.restore();
jest.resetModules();
});

it('uses the proper sub-list header level', async function () {
await generate('.', { splitBy: 'meta.docs.category' });
expect(readFileSync('README.md', 'utf8')).toMatchSnapshot();
});
});

describe('ignores case', function () {
beforeEach(function () {
mockFs({
Expand Down