Skip to content

Commit dba03f9

Browse files
authored
Improved the GS090 and GS100 custom-theme-setting rule wording (#817)
Both rules had passive titles that described the problem rather than the fix, and bodies that restated the title without pointing at the actual resolution. Pattern matches the GS110 rewrite shipped in #813.
1 parent b3bec0f commit dba03f9

4 files changed

Lines changed: 17 additions & 14 deletions

File tree

lib/ast-linter/rules/lint-no-unknown-custom-theme-settings.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module.exports = class NoUnknownCustomThemeSettings extends Rule {
66
const name = node.parts[1];
77
if (!this.isValidCustomThemeSettingReference(name)) {
88
this.log({
9-
message: `Missing Custom Theme Setting: "${name}"`,
9+
message: `Unknown {{@custom.${name}}} on line ${node.loc && node.loc.start.line}: ${this.sourceForNode(node)}`,
1010
line: node.loc && node.loc.start.line,
1111
column: node.loc && node.loc.start.column,
1212
source: this.sourceForNode(node)

lib/checks/100-custom-template-settings-usage.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ const ruleImplementations = {
2626
const config = Object.keys(theme.customSettings);
2727
const notUsedVariable = config.filter(x => !result.customThemeSettings.has(x));
2828

29-
if (notUsedVariable.length > 0) {
29+
notUsedVariable.forEach((name) => {
3030
log.failure({
31-
message: `Found unused variables: ${notUsedVariable.map(x => '@custom.' + x).join(', ')}`,
31+
message: `config.custom.${name} is declared but never referenced from a template`,
3232
ref: 'package.json'
3333
});
34-
}
34+
});
3535
}
3636
}
3737
};

lib/specs/v4.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,9 @@ let rules = {
172172
},
173173
'GS090-NO-UNKNOWN-CUSTOM-THEME-SETTINGS': {
174174
level: 'error',
175-
rule: 'An unknown custom theme setting has been used.',
176175
fatal: false,
177-
details: oneLineTrim`The custom theme setting should all be defined in the package.json <code>config.custom</code> object.`
176+
rule: 'Remove or correct the unknown <code>{{@custom}}</code> setting',
177+
details: oneLineTrim`Every <code>{{@custom.<i>name</i>}}</code> reference in a template needs a matching entry in <code>config.custom</code> in <code>package.json</code>. An unrecognised setting renders as empty. See the <a href="${docsBaseUrl}custom-settings/" target=_blank>custom settings documentation</a> for details.`
178178
},
179179
'GS090-NO-UNKNOWN-CUSTOM-THEME-SELECT-VALUE-IN-MATCH': {
180180
level: 'error',
@@ -209,8 +209,8 @@ let rules = {
209209

210210
'GS100-NO-UNUSED-CUSTOM-THEME-SETTING': {
211211
level: 'error',
212-
rule: 'A custom theme setting defined in <code>package.json</code> hasn\'t been used in any theme file.',
213-
details: oneLineTrim`Custom theme settings defined in <code>package.json</code> must be used at least once in the theme templates.`
212+
rule: 'Use or remove the unused <code>config.custom</code> setting',
213+
details: oneLineTrim`A setting declared in <code>config.custom</code> in <code>package.json</code> appears in the admin Customize panel but has no effect unless referenced from a template with <code>{{@custom.<i>name</i>}}</code>. Either use the setting from a template or drop it from <code>package.json</code>. See the <a href="${docsBaseUrl}custom-settings/" target=_blank>custom settings documentation</a> for details.`
214214
},
215215

216216
'GS050-CSS-KGCO': cssCardRule('callout', 'kg-callout-card'),

test/100-custom-template-settings-usage.test.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ describe('100 custom template settings usage', function () {
2121
expect(Object.keys(output.results.fail)).toEqual(['GS100-NO-UNUSED-CUSTOM-THEME-SETTING']);
2222

2323
utils.assertValidFailObject(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING']);
24-
expect(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures[0].message).toMatch(/color_scheme/);
25-
expect(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures[0].message).toMatch(/show_logo/);
24+
const messages = output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures.map(f => f.message);
25+
expect(messages.some(m => /color_scheme/.test(m))).toBe(true);
26+
expect(messages.some(m => /show_logo/.test(m))).toBe(true);
2627

2728
});
2829
});
@@ -99,8 +100,9 @@ describe('100 custom template settings usage', function () {
99100
expect(Object.keys(output.results.fail)).toEqual(['GS100-NO-UNUSED-CUSTOM-THEME-SETTING']);
100101

101102
utils.assertValidFailObject(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING']);
102-
expect(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures[0].message).toMatch(/color_scheme/);
103-
expect(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures[0].message).toMatch(/show_logo/);
103+
const messages = output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures.map(f => f.message);
104+
expect(messages.some(m => /color_scheme/.test(m))).toBe(true);
105+
expect(messages.some(m => /show_logo/.test(m))).toBe(true);
104106

105107
});
106108
});
@@ -177,8 +179,9 @@ describe('100 custom template settings usage', function () {
177179
expect(Object.keys(output.results.fail)).toEqual(['GS100-NO-UNUSED-CUSTOM-THEME-SETTING']);
178180

179181
utils.assertValidFailObject(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING']);
180-
expect(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures[0].message).toMatch(/color_scheme/);
181-
expect(output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures[0].message).toMatch(/show_logo/);
182+
const messages = output.results.fail['GS100-NO-UNUSED-CUSTOM-THEME-SETTING'].failures.map(f => f.message);
183+
expect(messages.some(m => /color_scheme/.test(m))).toBe(true);
184+
expect(messages.some(m => /show_logo/.test(m))).toBe(true);
182185

183186
});
184187
});

0 commit comments

Comments
 (0)