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

Split configs rule doc notice into separate lines by severity #311

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
4 changes: 3 additions & 1 deletion docs/examples/eslint-plugin-test/docs/rules/prefer-bar.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Enforce using bar (`test/prefer-bar`)

💼⚠️ This rule is enabled in the ✅ `recommended` config. This rule _warns_ in the 🎨 `stylistic` config.
💼 This rule is enabled in the ✅ `recommended` config.

⚠️ This rule _warns_ in the 🎨 `stylistic` config.

💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).

Expand Down
21 changes: 5 additions & 16 deletions lib/rule-doc-notices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { getLinkToRule, getUrlToRule } from './rule-link.js';
import { toSentenceCase, removeTrailingPeriod } from './string.js';

function severityToTerminology(severity: SEVERITY_TYPE) {
// Italics is used to draw attention to non-error severities since they are less common.
switch (severity) {
case SEVERITY_TYPE.error:
return 'is enabled';
Expand Down Expand Up @@ -54,9 +55,9 @@ function configsToNoticeSentence(
const term = severityToTerminology(severity);
const sentence =
configs.length > 1
? `This rule ${term} in the following ${configsLinkOrWord}: ${csv}.`
? `${EMOJI_CONFIG_FROM_SEVERITY[severity]} This rule ${term} in the following ${configsLinkOrWord}: ${csv}.`
: configs.length === 1
? `This rule ${term} in the ${csv} ${configLinkOrWord}.`
? `${EMOJI_CONFIG_FROM_SEVERITY[severity]} This rule ${term} in the ${csv} ${configLinkOrWord}.`
: undefined;

return sentence;
Expand Down Expand Up @@ -115,18 +116,6 @@ const RULE_NOTICES: {
);
}

// Use the emoji(s) for the severity levels this rule is set to in various configs.
const emojis: string[] = [];
if (configsError.length > 0) {
emojis.push(EMOJI_CONFIG_FROM_SEVERITY[SEVERITY_TYPE.error]);
}
if (configsWarn.length > 0) {
emojis.push(EMOJI_CONFIG_FROM_SEVERITY[SEVERITY_TYPE.warn]);
}
if (configsOff.length > 0) {
emojis.push(EMOJI_CONFIG_FROM_SEVERITY[SEVERITY_TYPE.off]);
}

const sentences = [
configsToNoticeSentence(
configsError,
Expand All @@ -151,9 +140,9 @@ const RULE_NOTICES: {
),
]
.filter(Boolean)
.join(' ');
.join('\n\n');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why two newlines instead of one?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently separate all notices with a blank line:

💼 This rule is enabled in the ✅ recommended config.

🚫 This rule is disabled in the other config.

Are you suggesting grouping these related notices together like:

💼 This rule is enabled in the ✅ recommended config.
🚫 This rule is disabled in the other config.

Copy link

@ljharb ljharb Nov 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly


return `${emojis.join('')} ${sentences}`;
return sentences;
},

// Deprecated notice has optional "replaced by" rules list.
Expand Down
8 changes: 6 additions & 2 deletions test/lib/generate/__snapshots__/configs-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ exports[`generate (configs) rules that are disabled or set to warn generates the
exports[`generate (configs) rules that are disabled or set to warn generates the documentation 4`] = `
"# Description of no-baz (\`test/no-baz\`)

💼🚫 This rule is enabled in the ✅ \`recommended\` config. This rule is _disabled_ in the \`other\` config.
💼 This rule is enabled in the ✅ \`recommended\` config.

🚫 This rule is _disabled_ in the \`other\` config.

<!-- end auto-generated rule header -->
"
Expand Down Expand Up @@ -211,7 +213,9 @@ exports[`generate (configs) rules that are disabled or set to warn, two configs
exports[`generate (configs) rules that are disabled or set to warn, two configs present generates the documentation 2`] = `
"# Description of no-foo (\`test/no-foo\`)

⚠️🚫 This rule _warns_ in the ✅ \`recommended\` config. This rule is _disabled_ in the ⌨️ \`typescript\` config.
⚠️ This rule _warns_ in the ✅ \`recommended\` config.

🚫 This rule is _disabled_ in the ⌨️ \`typescript\` config.

<!-- end auto-generated rule header -->
"
Expand Down