fix(export): apply the version gate to advanced options too

Picking an older Kitty is a promise the exported file will load in it,
and Advanced was writing its options whatever was selected. Twelve of
them, watcher and menu_map and action_alias among others, went out as
directives to a Kitty that has never heard of them, which it refuses on
the first unknown name.

The gate is the one the other sections already use, so they arrive
commented with the version that introduced them, like everything else.

An option that writes a line per entry was also losing all but the first
when held back, because only the first was being commented and the rest
were dropped without a word.

The suite runs every option through every supported version, which is how
both turned up.

Author: xcutiboo

src/services/kitty-generator.service.ts

@@ -195,6 +195,13 @@ export class KittyGeneratorService {
 				continue;
 			}
 
+			// Advanced was writing these whatever Kitty was selected, so picking an
+			// older one still produced a file it refuses on the first unknown name.
+			if (!this.versionService.isOptionAvailable(key)) {
+				this.collectUnavailableOption(key, advanced, defaults, lines);
+				continue;
+			}
+
 			this.collectConfigLine(key, advanced[key], defaults[key], lines);
 		}
 
@@ -308,10 +315,12 @@ export class KittyGeneratorService {
 		if (!requirement || !this.isDifferent(current[key], defaults[key])) return;
 
 		const formatted = this.formatValueForKey(key, current[key]);
-		if (formatted) {
-			lines.push(
-				`# ${formatted[0]}  # Requires Kitty >= ${requirement.minVersion}`,
-			);
+		if (!formatted) return;
+
+		// Every line, not just the first: an option like watcher writes one per
+		// entry, and commenting only the first dropped the rest without saying so.
+		for (const line of formatted) {
+			lines.push(`# ${line}  # Requires Kitty >= ${requirement.minVersion}`);
 		}
 	}
 

src/version-gating.spec.ts

@@ -0,0 +1,128 @@
+import { TestBed } from "@angular/core/testing";
+import { beforeEach, describe, expect, it } from "vitest";
+import { DEFAULT_KITTY_CONFIG } from "./models/kitty-defaults";
+import type { KittyConfigAST } from "./models/kitty-types";
+import { KittyGeneratorService } from "./services/kitty-generator.service";
+import { KittyVersionService } from "./services/kitty-version.service";
+
+/**
+ * Picking an older Kitty is a promise that the exported file will load in it.
+ * An option that release does not have has to arrive commented, carrying the
+ * version that introduced it, rather than as a directive that release will
+ * reject.
+ */
+describe("version gating", () => {
+	let generator: KittyGeneratorService;
+	let versions: KittyVersionService;
+
+	beforeEach(() => {
+		TestBed.resetTestingModule();
+		generator = TestBed.inject(KittyGeneratorService);
+		versions = TestBed.inject(KittyVersionService);
+	});
+
+	/** Every option in a section, paired with the section holding it. */
+	const OPTIONS = Object.entries(
+		DEFAULT_KITTY_CONFIG as unknown as Record<string, unknown>,
+	).flatMap(([section, values]) =>
+		values && typeof values === "object" && !Array.isArray(values)
+			? Object.keys(values as Record<string, unknown>).map(
+					(key) => [section, key] as const,
+				)
+			: [],
+	);
+
+	/** A different value of the same shape, so the option is written at all. */
+	function moved(value: unknown): unknown {
+		if (typeof value === "boolean") return !value;
+		if (typeof value === "number") return value + 1;
+		if (typeof value === "string") return value === "yes" ? "no" : `${value}x`;
+		if (Array.isArray(value)) return [...value, "extra"];
+		return value;
+	}
+
+	function everythingChanged(): KittyConfigAST {
+		const config = structuredClone(DEFAULT_KITTY_CONFIG) as unknown as Record<
+			string,
+			Record<string, unknown>
+		>;
+		for (const [section, key] of OPTIONS) {
+			const slice = config[section];
+			if (slice) slice[key] = moved(slice[key]);
+		}
+		return config as unknown as KittyConfigAST;
+	}
+
+	/** Directives only: a commented line is the thing being checked for. */
+	function liveDirectives(text: string): Set<string> {
+		const keys = new Set<string>();
+		for (const line of text.split("\n")) {
+			const trimmed = line.trim();
+			if (!trimmed || trimmed.startsWith("#")) continue;
+			const key = trimmed.split(/\s+/)[0];
+			if (key) keys.add(key);
+		}
+		return keys;
+	}
+
+	it("has something to gate on, or the rest of this proves nothing", () => {
+		const gated = OPTIONS.filter(([, key]) =>
+			versions.getVersionRequirement(key),
+		);
+		expect(gated.length).toBeGreaterThan(5);
+	});
+
+	it.each(TestBed.inject(KittyVersionService).versions.map((v) => v.version))(
+		"writes nothing Kitty %s would reject",
+		(version) => {
+			versions.setVersion(version);
+			const text = generator.generateConfig(everythingChanged());
+			const live = liveDirectives(text);
+
+			const tooNew = OPTIONS.filter(([, key]) => {
+				const requirement = versions.getVersionRequirement(key);
+				return requirement && !versions.isOptionAvailable(key) && live.has(key);
+			}).map(([, key]) => key);
+
+			expect(tooNew).toEqual([]);
+		},
+	);
+
+	/**
+	 * Modelled so the editor can offer a control, but written as part of another
+	 * directive rather than one of their own: `env read_from_shell` and
+	 * `confirm_os_window_close N count-background`.
+	 */
+	const FOLDED_INTO_ANOTHER_LINE = new Set([
+		"env_read_from_shell",
+		"confirm_os_window_close_count_background",
+	]);
+
+	it("still writes the options that release does have", () => {
+		versions.setVersion("0.48.0");
+		const live = liveDirectives(generator.generateConfig(everythingChanged()));
+
+		const missing = OPTIONS.filter(([, key]) => {
+			if (FOLDED_INTO_ANOTHER_LINE.has(key)) return false;
+			const requirement = versions.getVersionRequirement(key);
+			return requirement && versions.isOptionAvailable(key) && !live.has(key);
+		}).map(([, key]) => key);
+
+		expect(missing).toEqual([]);
+	});
+
+	it("says which Kitty an option it held back needs", () => {
+		versions.setVersion("0.15.0");
+		const text = generator.generateConfig(everythingChanged());
+
+		const gatedLines = text
+			.split("\n")
+			.filter((line) => line.includes("Requires Kitty >="));
+
+		expect(gatedLines.length).toBeGreaterThan(0);
+		for (const line of gatedLines) {
+			expect(line.trim().startsWith("#")).toBe(true);
+			expect(line).toMatch(/Requires Kitty >= \d+\.\d+/);
+		}
+	});
+});