From 839f9eb56c6c5b28faef80dce9fb75c40a143326 Mon Sep 17 00:00:00 2001 From: Bastiaan <17990055+bbassie@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:53:06 +0200 Subject: [PATCH] Use the config access API in the ctl config editor (#1059) serializedConfigToString now asks a control-located config instance whether each field is readable and writable instead of relying on a hard-coded list per command. Hidden and unknown fields are skipped, and field descriptions are written as comments above each value. Object fields such as factorio.settings are now included, so they are written as JSON, and configToKeyVal splits on the first = only so those values survive the round trip. Fixes #1051 Co-authored-by: Claude Opus 5.5 --- packages/ctl/src/commands_controller.ts | 4 +-- packages/ctl/src/commands_host.ts | 14 ++------ packages/ctl/src/commands_instance.ts | 14 ++------ packages/ctl/src/config_ops.ts | 43 +++++++++++------------ test/ctl/config_ops.test.js | 45 ++++++++++++++++++++++++- 5 files changed, 69 insertions(+), 51 deletions(-) diff --git a/packages/ctl/src/commands_controller.ts b/packages/ctl/src/commands_controller.ts index ad4a31cf..55417be4 100644 --- a/packages/ctl/src/commands_controller.ts +++ b/packages/ctl/src/commands_controller.ts @@ -127,9 +127,7 @@ controllerConfigCommands.add(new lib.Command({ throw new lib.CommandError(`No editor avalible. Checked CLI input, EDITOR and VISUAL env vars Try "ctl controller config edit "`); } - let allConfigElements = serializedConfigToString( - config, lib.ControllerConfig, {} - ); + let allConfigElements = serializedConfigToString(config, lib.ControllerConfig); await fs.writeFile(tmpFile, allConfigElements); let editorSpawn = child_process.spawn(editor, [tmpFile], { stdio: "inherit", diff --git a/packages/ctl/src/commands_host.ts b/packages/ctl/src/commands_host.ts index 77ab6936..a3dd8b7c 100644 --- a/packages/ctl/src/commands_host.ts +++ b/packages/ctl/src/commands_host.ts @@ -221,12 +221,7 @@ hostConfigCommands.add(new lib.Command({ throw new lib.CommandError(`No editor avalible. Checked CLI input, EDITOR and VISUAL env vars Try "ctl host config edit "`); } - let disallowedList = {"host.id": 0}; - let allConfigElements = serializedConfigToString( - config, - lib.HostConfig, - disallowedList - ); + let allConfigElements = serializedConfigToString(config, lib.HostConfig); await fs.writeFile(tmpFile, allConfigElements); let editorSpawn = child_process.spawn(editor, [tmpFile], { stdio: "inherit", @@ -247,13 +242,8 @@ hostConfigCommands.add(new lib.Command({ )); } catch (err) { // eslint-disable-next-line - print(`\n\n\nAttempt to set ${index} to ${final[index] || String(null)} failed; set back to previous value.`); + print(`Attempt to set ${index} to ${final[index] || String(null)} failed; set back to previous value.`); // If the string is empty, it's better to just print "" instead of nothing - print("This message shouldn't normally appear; if the below message does not indicate it"); - print("was a user mistake, please report it to the clustorio devs."); - // added this because it could be a missed entry in disallowedList - // i've added all the vanilla clustorio configs, but there may be - // some modded ones that need to be added. print(err); } } diff --git a/packages/ctl/src/commands_instance.ts b/packages/ctl/src/commands_instance.ts index bedd78eb..63440875 100644 --- a/packages/ctl/src/commands_instance.ts +++ b/packages/ctl/src/commands_instance.ts @@ -158,12 +158,7 @@ instanceConfigCommands.add(new lib.Command({ throw new lib.CommandError(`No editor avalible. Checked CLI input, EDITOR and VISUAL env vars Try "ctl instance config edit "`); } - let disallowedList = {"instance.id": 0, "instance.assigned_host": 0, "factorio.settings": 0}; - let allConfigElements = serializedConfigToString( - config, - lib.InstanceConfig, - disallowedList - ); + let allConfigElements = serializedConfigToString(config, lib.InstanceConfig); await fs.writeFile(tmpFile, allConfigElements); let editorSpawn = child_process.spawn(editor, [tmpFile], { stdio: "inherit", @@ -185,13 +180,8 @@ instanceConfigCommands.add(new lib.Command({ )); } catch (err) { // eslint-disable-next-line - print(`\n\n\nAttempt to set ${index} to ${final[index] || String(null)} failed; set back to previous value.`); + print(`Attempt to set ${index} to ${final[index] || String(null)} failed; set back to previous value.`); // If the string is empty, it's better to just print "" instead of nothing - print("This message shouldn't normally appear; if the below message does not indicate it"); - print("was a user mistake, please report it to the clustorio devs."); - // added this because it could be a missed entry in disallowedList - // i've added all the vanilla clustorio configs, but there may be - // some modded ones that need to be added. print(err); } } diff --git a/packages/ctl/src/config_ops.ts b/packages/ctl/src/config_ops.ts index 09299657..965b247a 100644 --- a/packages/ctl/src/config_ops.ts +++ b/packages/ctl/src/config_ops.ts @@ -15,18 +15,14 @@ export async function configToKeyVal(data: string) { // the last filter removes empty elements left by the first. Not done on one line due to readability. for (let index in filtered) { if (index in filtered) { - let split = filtered[index].split("="); - let finalIndex = split[0].trim(); - // split on the = we added earlier, giving us both value and key - let part = ""; - try { - part = split[1].trim(); - // it's a string if we can read it - } catch (err) { - // if we can't read it, it's a empty field and therefor null - part = ""; + // split on the first = only, values may contain = themselves + let split = filtered[index].indexOf("="); + if (split === -1) { + // no value given, it's a empty field and therefor null + final[filtered[index].trim()] = ""; + continue; } - final[finalIndex] = part; + final[filtered[index].slice(0, split).trim()] = filtered[index].slice(split + 1).trim(); } } return final; @@ -34,25 +30,26 @@ export async function configToKeyVal(data: string) { export function serializedConfigToString( serializedConfig: lib.ConfigSchema, - configGroup: typeof lib.ControllerConfig | typeof lib.HostConfig | typeof lib.InstanceConfig, - disallowedList: Record, + configClass: typeof lib.ControllerConfig | typeof lib.HostConfig | typeof lib.InstanceConfig, ) { + const config = configClass.fromJSON({}, "control"); let allConfigElements = ""; for (let [name, value] of Object.entries(serializedConfig)) { - if (name in disallowedList) { + const def = (configClass.fieldDefinitions as lib.ConfigDefs)[name]; + try { + if (!config.canAccess(name, lib.ConfigAccess.readWrite) || def.hidden) { + continue; + } + } catch (err) { + // Field does not exist in the config class continue; } - let desc = ""; - try { - desc += (configGroup.fieldDefinitions as any)[name]!.description; - } catch (err) { - desc += "No description found"; - } - // split onto two lines for readability and es-lint - if (String(value) === "null") { + if (value === null) { value = ""; + } else if (typeof value === "object") { + value = JSON.stringify(value); } - allConfigElements += `${name} = ${value}\n\n`; + allConfigElements += `# ${def.description ?? "No description found"}\n${name} = ${value}\n\n`; } return allConfigElements; } diff --git a/test/ctl/config_ops.test.js b/test/ctl/config_ops.test.js index 5d3921e1..ff683132 100644 --- a/test/ctl/config_ops.test.js +++ b/test/ctl/config_ops.test.js @@ -1,5 +1,6 @@ import assert from "node:assert/strict"; -import { configToKeyVal } from "@clusterio/ctl/dist/node/src/config_ops.js"; +import * as lib from "@clusterio/lib"; +import { configToKeyVal, serializedConfigToString } from "@clusterio/ctl/dist/node/src/config_ops.js"; describe("ctl/config_ops", function() { it("preserves full field names with LF and CRLF input", async function() { @@ -18,4 +19,46 @@ describe("ctl/config_ops", function() { }); } }); + + it("keeps = inside values", async function() { + assert.deepEqual(await configToKeyVal("a = {\"b\":\"c=d\"}\nempty =\nmissing\n"), { + "a": "{\"b\":\"c=d\"}", + "empty": "", + "missing": "", + }); + }); + + describe("serializedConfigToString()", function() { + const serialized = new lib.InstanceConfig("controller").toRemote("control"); + + it("writes descriptions above each value", function() { + const output = serializedConfigToString(serialized, lib.InstanceConfig); + const description = lib.InstanceConfig.fieldDefinitions["instance.auto_start"].description; + assert(output.includes(`# ${description}\ninstance.auto_start = false\n\n`)); + assert(output.includes("# No description found\ninstance.name = New Instance\n\n")); + }); + + it("skips hidden, inaccessible and unknown fields", function() { + const output = serializedConfigToString( + { ...serialized, "unknown.field": "value" }, lib.InstanceConfig + ); + const names = [...output.matchAll(/^([^#\n][^=]*) =/gm)].map(match => match[1]); + assert(!names.includes("instance.id")); + assert(!names.includes("instance.assigned_host")); + assert(!names.includes("unknown.field")); + for (const name of names) { + const def = lib.InstanceConfig.fieldDefinitions[name]; + assert(!def.hidden, `${name} is hidden`); + } + }); + + it("round trips object values through configToKeyVal", async function() { + const settings = { name: "a=b", tags: ["x"] }; + const output = serializedConfigToString( + { ...serialized, "factorio.settings": settings }, lib.InstanceConfig + ); + const parsed = await configToKeyVal(output); + assert.deepEqual(JSON.parse(parsed["factorio.settings"]), settings); + }); + }); });