textlint
Isn't it possible to enable & disable specific preset by merge-config?
As background, I am trying to provide a mechanism to bundle multiple rule presets into one Chrome extension and let users choose which one they want to use.
I tried to implement this using merge-config command while reading the documentation on the following page,
- https://github.com/textlint/editor/tree/master/packages/%40textlint/script-compiler
like this:
textlintWorker.postMessage({
command: "merge-config",
textlintrc: {
rules: {
"preset-ja-technical-writing": true,
"preset-japanese": false
},
},
});
but the above code did not work as intended to toggle disabling/enabling of the entire preset.
Is this currently not feasible without adding individual settings to switch disable/enable for each preset for each rule, as shown below?
textlintWorker.postMessage({
command: "merge-config",
textlintrc: {
rules: {
"preset-ja-technical-writing": {
"max-comma": {
max: Number.MAX_SAFE_INTEGER,
},
"sentence-length": {
max: Number.MAX_SAFE_INTEGER,
},
"max-ten": {
max: Number.MAX_SAFE_INTEGER,
},
"max-kanji-continuous-len": {
max: Number.MAX_SAFE_INTEGER,
allow: [],
},
"arabic-kanji-numbers": false,
// ...
},
"preset-japanese": {
// ...
},
},
},
});
In this case, though, it makes no sense to use presets in the first place, and it would be easier to use the individual rules without presets.
In addition, I would like to take this opportunity to ask a follow-up question: isn't it possible to "disable a specific rule" in merge-config? I think that just mergeing false for each individual ruleId element won't work. By this issue, the above example also includes a workaround that sets a suitably large value for max.
Probably, It's a bug
In textlint kernal, presets are expanded as separate rules and then loaded.
I think the current implementation is failing to override the merge-config command because it has the wrong rule id to load.
https://github.com/textlint/editor/blob/d5d5a875ed3fc4a720d056363c7f8bb61937ecf4/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts#L110-L126
maybe like this.
diff --git a/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts b/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts
index d9d3732..462ca07 100644
--- a/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts
+++ b/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts
@@ -6,6 +6,7 @@ import type {
} from "@textlint/config-loader";
import type { TextlintFixResult, TextlintResult } from "@textlint/types";
import { TextlintScriptMetadata } from "@textlint/script-parser";
+import { normalizeTextlintPresetSubRuleKey } from "@textlint/utils";
export type MessageId = string;
export type TextlintWorkerCommandLint = {
@@ -108,11 +109,19 @@ ${config.plugins.map(createPluginImportCode).join("\n")}
const kernel = new TextlintKernel();
const rules = ${stringify(
config.rules.map((rule, index) => {
- return {
- ruleId: rule.ruleId,
- rule: `!__moduleInterop(__rule${index})__!`,
- options: rule.options
- };
+ if (rule.type === "RuleInPreset") {
+ return {
+ ruleId: normalizeTextlintPresetSubRuleKey({ preset: rule.ruleId, rule: rule.ruleKey }),
+ rule: `!__moduleInterop(__rule${index})__!`,
+ options: rule.options
+ };
+ } else {
+ return {
+ ruleId: rule.ruleId,
+ rule: `!__moduleInterop(__rule${index})__!`,
+ options: rule.options
+ };
+ }
})
)};
const filterRules = ${stringify(
merge-config already use {preset id}/{child rule id}
https://github.com/textlint/editor/blob/d5d5a875ed3fc4a720d056363c7f8bb61937ecf4/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts#L152-L172 https://github.com/textlint/editor/blob/e7f734eb2e1e44d85592159f37d3a83ab50f7966/packages/@textlint/config-partial-parser/src/parser.ts#L55-L85
Oh, I misread that.
In textlint kernal, presets are expanded as separate rules and then loaded.
in script-compiler https://github.com/textlint/editor/blob/d5d5a875ed3fc4a720d056363c7f8bb61937ecf4/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts#L152-L172 in textlint https://github.com/textlint/textlint/blob/e184d1f33340bb3fc0bc13c50f62d8feb6ecae8a/packages/%40textlint/config-loader/src/loader.ts#L215-L223
Probably, We need to treat preset-abc: false as preset-abc/a: false, preset-abc/b: false .... in merge-config command as special case.
https://github.com/textlint/editor/blob/d5d5a875ed3fc4a720d056363c7f8bb61937ecf4/packages/@textlint/script-compiler/src/CodeGenerator/worker-codegen.ts#L152-L172
In addition, I would like to take this opportunity to ask a follow-up question: isn't it possible to "disable a specific rule" in merge-config?
It is possible that the process of excluding rules with an option of false from the config may be missing.
The "merge-config" has not been tested and should be re-created.
This is a rough image, but I think we will need the following logic to merge configs.
export const assignConfig = (currentConfig: {
plugins: TextlintKernelPlugin[];
rules: TextlintKernelRule[];
filterRules: TextlintKernelFilterRule[];
}, newConfig: TextlintRcConfig) => {
const { rules, plugins, filterRules } = parseOptionsFromConfig(newConfig);
const mergeItemsOptionById = (currentItems: TextlintStaticOptionRule[], newItems: TextlintStaticOptionRule[]) => {
return newItems.flatMap((newItem) => {
const currentItem = currentItems.find((item) => item.ruleId === newItem.ruleId);
if (currentItem) {
return {
...currentItem,
options: newItem.options
};
}
return newItem;
})
};
const mergePluginById = (currentItems: TextlintStaticOptionPlugin[], newItems: TextlintStaticOptionPlugin[]) => {
return newItems.flatMap((newItem) => {
const currentItem = currentItems.find((item) => item.pluginId === newItem.pluginId);
if (currentItem) {
return {
...currentItem,
options: newItem.options
};
}
return newItem;
})
}
// if options is false, this will be disabled
return {
rules: mergeItemsOptionById(currentConfig.rules, rules),
filterRules: mergeItemsOptionById(currentConfig.filterRules, filterRules),
plugins: mergePluginById(currentConfig.plugins, plugins)
};
}
It is easier to test if implemented as a runtime library like @textlint/config-partial-parser rather than embedding it directly.
I see. I like the idea of expanding { presetA: false } that way.
Reviewing from the design of config, I see there are two options on how to handle { ruleA: false } here.
One is to leave the current config as it is in its current design and regard false as a special instruction to delete the element when merge is performed, as you described.
The other is to design the config so that false is a special setting that disables the rule, so that when you merge, you do nothing special and false is the setting value of the rule. This way, I think it will work well not only when merge, but also when writing { ruleA: false } in .textlintrc, so there are fewer surprises and more symmetry.
https://github.com/textlint/editor/issues/87#issuecomment-1874879652 This comment is probably wrong and hide it.
- In textlint:
- @textlint/config-loader expands preset to rules before passing config to kernel
- ℹ️ This limitation come from that kernel can not use Node.js API like
node:fs
- In editor script:
- Already expanded rules exists
- editor script can not list rules from preset name unless pass more info
The other is to design the config so that false is a special setting that disables the rule, so that when you merge, you do nothing special and false is the setting value of the rule. This way, I think it will work well not only when merge, but also when writing { ruleA: false } in .textlintrc, so there are fewer surprises and more symmetry.
Can you explain the detail? I couldn't imagine it imho.
The other is to design the config so that false is a special setting that disables the rule, so that when you merge, you do nothing special and false is the setting value of the rule. This way, I think it will work well not only when merge, but also when writing { ruleA: false } in .textlintrc, so there are fewer surprises and more symmetry.
Can you explain the detail? I couldn't imagine it imho.
const config1 = {
rules: {
a: { /* ... */ }
}
};
const config2 = {
rules: {
a: false
}
};
const result = mergeConfig(config1, config2)
In the code above, I'm talking about whether the result should be { rules: {} } or { rules: { a: false } }.
In the latter case, the responsibility for handling the false would be of the config value handler, not the merge handler.