Preserve VS Code settings comments during merge

2026-04-23 16:12:30 -04:00
parent adfcb83ab6
commit 1a2f1510bf
4 changed files with 592 additions and 23 deletions
--- a/install/bootstrap.sh
+++ b/install/bootstrap.sh
@@ -45,13 +45,16 @@ find_node_bin() {
 link_path() {
  local target="$1"
  local link_path="$2"
+  local resolved_target
+
+  resolved_target="$(resolve_dir "$target")"

  ensure_parent_dir "$link_path"

  if [[ -L "$link_path" ]]; then
    local existing_target
    existing_target="$(cd -- "$(dirname -- "$link_path")" && resolve_dir "$(readlink "$link_path")")"
-    if [[ "$existing_target" == "$target" ]]; then
+    if [[ "$existing_target" == "$resolved_target" ]]; then
      return 0
    fi
    printf 'Refusing to replace existing symlink %s -> %s\n' "$link_path" "$existing_target" >&2
--- a/install/merge-vscode-settings.mjs
+++ b/install/merge-vscode-settings.mjs
@@ -2,6 +2,7 @@

 import fs from 'node:fs';
 import path from 'node:path';
+import { isDeepStrictEqual } from 'node:util';

 function usage() {
  console.error(
@@ -191,34 +192,375 @@ function parseJsonc(input, label) {
  return parsed;
 }

+function parseJsoncAst(input, label) {
+  let index = 0;
+
+  function fail(message) {
+    throw new Error(`Failed to parse ${label}: ${message}`);
+  }
+
+  function skipTrivia() {
+    while (index < input.length) {
+      const char = input[index];
+      const next = input[index + 1];
+
+      if (/\s/.test(char)) {
+        index += 1;
+        continue;
+      }
+
+      if (char === '/' && next === '/') {
+        index += 2;
+        while (index < input.length && input[index] !== '\n') {
+          index += 1;
+        }
+        continue;
+      }
+
+      if (char === '/' && next === '*') {
+        index += 2;
+        while (index < input.length && !(input[index] === '*' && input[index + 1] === '/')) {
+          index += 1;
+        }
+        if (index >= input.length) {
+          fail('unterminated block comment');
+        }
+        index += 2;
+        continue;
+      }
+
+      break;
+    }
+  }
+
+  function parseStringNode() {
+    const start = index;
+    index += 1;
+
+    while (index < input.length) {
+      const char = input[index];
+
+      if (char === '\\') {
+        index += 2;
+        continue;
+      }
+
+      if (char === '"') {
+        index += 1;
+        const raw = input.slice(start, index);
+        return {
+          type: 'string',
+          start,
+          end: index,
+          value: JSON.parse(raw),
+        };
+      }
+
+      index += 1;
+    }
+
+    fail('unterminated string literal');
+  }
+
+  function parseLiteralNode(expectedText, value) {
+    const start = index;
+    if (!input.startsWith(expectedText, index)) {
+      fail(`expected ${expectedText}`);
+    }
+    index += expectedText.length;
+    return {
+      type: typeof value,
+      start,
+      end: index,
+      value,
+    };
+  }
+
+  function parseNumberNode() {
+    const match = /^-?(?:0|[1-9]\d*)(?:\.\d+)?(?:[eE][+-]?\d+)?/.exec(input.slice(index));
+    if (!match) {
+      fail(`invalid number at offset ${index}`);
+    }
+
+    const start = index;
+    index += match[0].length;
+    return {
+      type: 'number',
+      start,
+      end: index,
+      value: Number(match[0]),
+    };
+  }
+
+  function parseArrayNode() {
+    const start = index;
+    index += 1;
+    const elements = [];
+
+    skipTrivia();
+    while (index < input.length && input[index] !== ']') {
+      const valueNode = parseValueNode();
+      elements.push(valueNode);
+      skipTrivia();
+
+      if (input[index] === ',') {
+        index += 1;
+        skipTrivia();
+        continue;
+      }
+
+      if (input[index] !== ']') {
+        fail(`expected ',' or ']' at offset ${index}`);
+      }
+    }
+
+    if (input[index] !== ']') {
+      fail('unterminated array');
+    }
+
+    index += 1;
+    return {
+      type: 'array',
+      start,
+      end: index,
+      elements,
+      value: elements.map((element) => element.value),
+    };
+  }
+
+  function parseObjectNode() {
+    const start = index;
+    index += 1;
+    const properties = [];
+    const value = {};
+
+    skipTrivia();
+    while (index < input.length && input[index] !== '}') {
+      if (input[index] !== '"') {
+        fail(`expected string property name at offset ${index}`);
+      }
+
+      const keyNode = parseStringNode();
+      const key = keyNode.value;
+      skipTrivia();
+
+      if (input[index] !== ':') {
+        fail(`expected ':' after property name at offset ${index}`);
+      }
+
+      index += 1;
+      skipTrivia();
+
+      const valueNode = parseValueNode();
+      const property = {
+        key,
+        keyNode,
+        value: valueNode,
+        hasTrailingComma: false,
+      };
+
+      properties.push(property);
+      value[key] = valueNode.value;
+
+      skipTrivia();
+      if (input[index] === ',') {
+        property.hasTrailingComma = true;
+        index += 1;
+        skipTrivia();
+        continue;
+      }
+
+      if (input[index] !== '}') {
+        fail(`expected ',' or '}' at offset ${index}`);
+      }
+    }
+
+    if (input[index] !== '}') {
+      fail('unterminated object');
+    }
+
+    index += 1;
+    return {
+      type: 'object',
+      start,
+      end: index,
+      properties,
+      value,
+    };
+  }
+
+  function parseValueNode() {
+    skipTrivia();
+
+    const char = input[index];
+    if (char === '{') {
+      return parseObjectNode();
+    }
+    if (char === '[') {
+      return parseArrayNode();
+    }
+    if (char === '"') {
+      return parseStringNode();
+    }
+    if (char === 't') {
+      return parseLiteralNode('true', true);
+    }
+    if (char === 'f') {
+      return parseLiteralNode('false', false);
+    }
+    if (char === 'n') {
+      return parseLiteralNode('null', null);
+    }
+    if (char === '-' || /\d/.test(char ?? '')) {
+      return parseNumberNode();
+    }
+
+    fail(`unexpected token at offset ${index}`);
+  }
+
+  skipTrivia();
+  const root = parseValueNode();
+  skipTrivia();
+
+  if (index !== input.length) {
+    fail(`unexpected trailing content at offset ${index}`);
+  }
+
+  if (root.type !== 'object') {
+    throw new Error(`${label} must contain a JSON object at the root.`);
+  }
+
+  return root;
+}
+
 function isPlainObject(value) {
  return value !== null && typeof value === 'object' && !Array.isArray(value);
 }

-function cloneValue(value) {
-  if (Array.isArray(value)) {
-    return value.map((item) => cloneValue(item));
+function detectIndentUnit(input) {
+  const matches = input.match(/^( +|\t+)\S/m);
+  if (!matches) {
+    return '  ';
  }
-  if (isPlainObject(value)) {
-    return Object.fromEntries(
-      Object.entries(value).map(([key, nested]) => [key, cloneValue(nested)])
-    );
-  }
-  return value;
+
+  return matches[1];
 }

-function deepMerge(target, source) {
-  const merged = { ...target };
+function detectEol(input) {
+  return input.includes('\r\n') ? '\r\n' : '\n';
+}

-  for (const [key, value] of Object.entries(source)) {
-    if (isPlainObject(value) && isPlainObject(merged[key])) {
-      merged[key] = deepMerge(merged[key], value);
-      continue;
-    }
-    merged[key] = cloneValue(value);
+function lineStartIndex(input, index) {
+  const newlineIndex = input.lastIndexOf('\n', index - 1);
+  return newlineIndex === -1 ? 0 : newlineIndex + 1;
+}
+
+function lineIndentAt(input, index) {
+  const start = lineStartIndex(input, index);
+  let end = start;
+  while (end < input.length && (input[end] === ' ' || input[end] === '\t')) {
+    end += 1;
+  }
+  return input.slice(start, end);
+}
+
+function renderValue(value, propertyIndent, indentUnit, eol) {
+  const raw = JSON.stringify(value, null, indentUnit);
+  if (!raw.includes('\n')) {
+    return raw;
  }

-  return merged;
+  return raw
+    .split('\n')
+    .map((line, index) => (index === 0 ? line : `${propertyIndent}${line}`))
+    .join(eol);
+}
+
+function renderProperty(key, value, propertyIndent, indentUnit, eol) {
+  return `${JSON.stringify(key)}: ${renderValue(value, propertyIndent, indentUnit, eol)}`;
+}
+
+function getObjectChildIndent(input, objectNode, indentUnit) {
+  if (objectNode.properties.length > 0) {
+    return lineIndentAt(input, objectNode.properties[0].keyNode.start);
+  }
+
+  return `${lineIndentAt(input, objectNode.start)}${indentUnit}`;
+}
+
+function buildMergeEdits(input, objectNode, managedSettings, indentUnit, eol, edits) {
+  const missingEntries = [];
+
+  for (const [key, managedValue] of Object.entries(managedSettings)) {
+    const property = objectNode.properties.find((candidate) => candidate.key === key);
+
+    if (!property) {
+      missingEntries.push([key, managedValue]);
+      continue;
+    }
+
+    if (isPlainObject(managedValue) && property.value.type === 'object') {
+      buildMergeEdits(input, property.value, managedValue, indentUnit, eol, edits);
+      continue;
+    }
+
+    if (!isDeepStrictEqual(property.value.value, managedValue)) {
+      const propertyIndent = lineIndentAt(input, property.keyNode.start);
+      edits.push({
+        start: property.value.start,
+        end: property.value.end,
+        text: renderValue(managedValue, propertyIndent, indentUnit, eol),
+      });
+    }
+  }
+
+  if (missingEntries.length === 0) {
+    return;
+  }
+
+  const propertyIndent = getObjectChildIndent(input, objectNode, indentUnit);
+  const closingIndent = lineIndentAt(input, objectNode.end);
+  const renderedProperties = missingEntries
+    .map(([key, value]) => `${propertyIndent}${renderProperty(key, value, propertyIndent, indentUnit, eol)}`)
+    .join(`,${eol}`);
+
+  if (objectNode.properties.length === 0) {
+    edits.push({
+      start: objectNode.start + 1,
+      end: objectNode.end,
+      text: `${eol}${renderedProperties}${eol}${closingIndent}`,
+    });
+    return;
+  }
+
+  const lastProperty = objectNode.properties[objectNode.properties.length - 1];
+  if (!lastProperty.hasTrailingComma) {
+    edits.push({
+      start: lastProperty.value.end,
+      end: lastProperty.value.end,
+      text: ',',
+    });
+  }
+
+  const closingLineStart = lineStartIndex(input, objectNode.end);
+  const insertBeforeClosingLine = closingLineStart > lastProperty.value.end;
+
+  edits.push({
+    start: insertBeforeClosingLine ? closingLineStart : objectNode.end,
+    end: insertBeforeClosingLine ? closingLineStart : objectNode.end,
+    text: insertBeforeClosingLine
+      ? `${renderedProperties}${eol}`
+      : `${eol}${renderedProperties}${eol}${closingIndent}`,
+  });
+}
+
+function applyEdits(input, edits) {
+  return edits
+    .sort((left, right) => right.start - left.start || right.end - left.end)
+    .reduce(
+      (text, edit) => `${text.slice(0, edit.start)}${edit.text}${text.slice(edit.end)}`,
+      input
+    );
 }

 function main() {
@@ -230,9 +572,15 @@ function main() {
  const targetText = fs.existsSync(options.target)
    ? fs.readFileSync(options.target, 'utf8')
    : '{}\n';
-  const currentSettings = parseJsonc(targetText, options.target);
-  const mergedSettings = deepMerge(currentSettings, managedSettings);
-  const output = `${JSON.stringify(mergedSettings, null, 2)}\n`;
+  const targetAst = parseJsoncAst(targetText, options.target);
+  const indentUnit = detectIndentUnit(targetText);
+  const eol = detectEol(targetText);
+  const edits = [];
+
+  buildMergeEdits(targetText, targetAst, managedSettings, indentUnit, eol, edits);
+  const output = edits.length === 0 ? targetText : applyEdits(targetText, edits);
+
+  parseJsonc(output, options.target);

  fs.mkdirSync(path.dirname(options.target), { recursive: true });

--- a/install/merge-vscode-settings.test.mjs
+++ b/install/merge-vscode-settings.test.mjs
@@ -0,0 +1,216 @@
+import assert from 'node:assert/strict';
+import { spawnSync } from 'node:child_process';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import test from 'node:test';
+import { fileURLToPath } from 'node:url';
+
+const scriptDir = path.dirname(fileURLToPath(import.meta.url));
+const repoRoot = path.resolve(scriptDir, '..');
+const mergeScript = path.join(scriptDir, 'merge-vscode-settings.mjs');
+const templateFile = path.join(repoRoot, 'config', 'vscode', 'settings.template.jsonc');
+
+function stripJsonComments(input) {
+  let output = '';
+  let inString = false;
+  let escaping = false;
+  let inLineComment = false;
+  let inBlockComment = false;
+
+  for (let index = 0; index < input.length; index += 1) {
+    const char = input[index];
+    const next = input[index + 1];
+
+    if (inLineComment) {
+      if (char === '\n') {
+        inLineComment = false;
+        output += char;
+      }
+      continue;
+    }
+
+    if (inBlockComment) {
+      if (char === '*' && next === '/') {
+        inBlockComment = false;
+        index += 1;
+      } else if (char === '\n' || char === '\r') {
+        output += char;
+      }
+      continue;
+    }
+
+    if (inString) {
+      output += char;
+      if (escaping) {
+        escaping = false;
+      } else if (char === '\\') {
+        escaping = true;
+      } else if (char === '"') {
+        inString = false;
+      }
+      continue;
+    }
+
+    if (char === '"') {
+      inString = true;
+      output += char;
+      continue;
+    }
+
+    if (char === '/' && next === '/') {
+      inLineComment = true;
+      index += 1;
+      continue;
+    }
+
+    if (char === '/' && next === '*') {
+      inBlockComment = true;
+      index += 1;
+      continue;
+    }
+
+    output += char;
+  }
+
+  return output;
+}
+
+function stripTrailingCommas(input) {
+  let output = '';
+  let inString = false;
+  let escaping = false;
+
+  for (let index = 0; index < input.length; index += 1) {
+    const char = input[index];
+
+    if (inString) {
+      output += char;
+      if (escaping) {
+        escaping = false;
+      } else if (char === '\\') {
+        escaping = true;
+      } else if (char === '"') {
+        inString = false;
+      }
+      continue;
+    }
+
+    if (char === '"') {
+      inString = true;
+      output += char;
+      continue;
+    }
+
+    if (char === ',') {
+      let lookahead = index + 1;
+      while (lookahead < input.length && /\s/.test(input[lookahead])) {
+        lookahead += 1;
+      }
+      if (input[lookahead] === '}' || input[lookahead] === ']') {
+        continue;
+      }
+    }
+
+    output += char;
+  }
+
+  return output;
+}
+
+function parseJsonc(input) {
+  return JSON.parse(stripTrailingCommas(stripJsonComments(input)));
+}
+
+function runMerge(targetFile) {
+  return spawnSync(
+    process.execPath,
+    [
+      mergeScript,
+      '--target',
+      targetFile,
+      '--template',
+      templateFile,
+      '--set',
+      'COPILOT_RESOURCES_HOME=/repo/home',
+    ],
+    {
+      cwd: repoRoot,
+      encoding: 'utf8',
+    }
+  );
+}
+
+test('preserves comments and custom nested entries while inserting managed settings', () => {
+  const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'copilot-settings-'));
+  const targetFile = path.join(tempDir, 'settings.json');
+
+  fs.writeFileSync(
+    targetFile,
+    `{
+  // keep this comment
+  "workbench.colorTheme": "GitHub Dark Mode",
+
+  // preserve nested comments
+  "chat.agentFilesLocations": {
+    // custom agents stay
+    "/custom/agents": true
+  },
+
+  "chat.instructionsFilesLocations": {
+    "/old/instructions": true,
+    "~/.claude/rules": false,
+  }
+}
+`,
+    'utf8'
+  );
+
+  const result = runMerge(targetFile);
+  assert.equal(result.status, 0, result.stderr);
+
+  const output = fs.readFileSync(targetFile, 'utf8');
+  assert.match(output, /\/\/ keep this comment/);
+  assert.match(output, /\/\/ preserve nested comments/);
+  assert.match(output, /\/\/ custom agents stay/);
+  assert.match(output, /"\/custom\/agents": true/);
+  assert.match(output, /"\/repo\/home\/resources\/agents": true/);
+
+  const parsed = parseJsonc(output);
+  assert.equal(parsed['workbench.colorTheme'], 'GitHub Dark Mode');
+  assert.equal(parsed['chat.agentFilesLocations']['/custom/agents'], true);
+  assert.equal(parsed['chat.agentFilesLocations']['/repo/home/resources/agents'], true);
+  assert.equal(parsed['chat.agentFilesLocations']['~/.copilot/agents'], true);
+  assert.equal(parsed['chat.instructionsFilesLocations']['/old/instructions'], true);
+  assert.equal(parsed['chat.instructionsFilesLocations']['/repo/home/resources/instructions'], true);
+  assert.equal(parsed['chat.instructionsFilesLocations']['~/.claude/rules'], true);
+});
+
+test('second run is idempotent and keeps the file text unchanged', () => {
+  const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'copilot-settings-'));
+  const targetFile = path.join(tempDir, 'settings.json');
+
+  fs.writeFileSync(
+    targetFile,
+    `{
+  // user comment
+  "chat.promptFilesLocations": {
+    "/custom/prompts": true
+  }
+}
+`,
+    'utf8'
+  );
+
+  const firstRun = runMerge(targetFile);
+  assert.equal(firstRun.status, 0, firstRun.stderr);
+  const firstOutput = fs.readFileSync(targetFile, 'utf8');
+
+  const secondRun = runMerge(targetFile);
+  assert.equal(secondRun.status, 0, secondRun.stderr);
+  assert.match(secondRun.stdout, /already up to date/);
+
+  const secondOutput = fs.readFileSync(targetFile, 'utf8');
+  assert.equal(secondOutput, firstOutput);
+  assert.match(secondOutput, /\/\/ user comment/);
+});