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 <noreply@anthropic.com>

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 <editor of choice>"`);
 		}
-		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",

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 <editor of choice>"`);
 		}
-		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);
 				}
 			}

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 <editor of choice>"`);
 		}
-		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);
 				}
 			}

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<string, unknown>,
+	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<any>)[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;
 }

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);
+		});
+	});
 });