feat(cli): polite deep merge for settings.json and support JSONC by RbBtSn0w · Pull Request #1863 · github/spec-kit

RbBtSn0w · 2026-03-16T03:31:00Z

Description

This PR addresses issue #1799, where the Specify CLI would overwrite the user's existing .vscode/settings.json file during initialization or upgrades, leading to data loss of personal preferences.

Key Changes

JSONC Support: Added a strip_json_comments utility to handle JSON files with single-line (//) and multi-line (/* */) comments, which are common in VSCode configuration files.
Polite Deep Merge: Implemented deep_merge_polite which ensures that:
- New recommended settings are added.
- Existing user settings are preserved and not overwritten by template defaults.
- Nested dictionaries are merged recursively to maintain structure.
Object Validation: Added basic validation to ensure existing JSON contents are objects (dictionaries) before attempting a merge.
Unit Tests: Added tests/test_merge.py covering comment stripping, polite merging logic, and error handling for invalid/non-dict JSON files.
Changelog: Updated CHANGELOG.md with these improvements.

Verification

Verified that comments in settings.json no longer cause the merge to fail and revert to a full overwrite.
Verified that existing user settings (e.g., editor.fontSize) are kept intact.
Verified all new tests pass with uv run pytest tests/test_merge.py.

Fixes #1799

…JSONC

Copilot

Pull request overview

This PR updates Specify CLI’s .vscode/settings.json handling to avoid overwriting user settings during init/upgrade by parsing JSON-with-comments and performing a “polite” deep merge.

Changes:

Added strip_json_comments() and used it when reading template and existing VSCode settings.
Implemented merge_json_files() with a deep-merge policy that preserves existing (user) values while adding new keys.
Added unit tests for comment stripping and merge fallback behavior; updated changelog entries.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`src/specify_cli/__init__.py`	Adds JSONC-ish parsing and polite deep-merge logic for `.vscode/settings.json`.
`tests/test_merge.py`	Introduces tests for comment stripping and merge behavior/fallbacks.
`CHANGELOG.md`	Documents the new merge/JSONC support behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/test_merge.py

+    assert parsed["a"] == 1
+    assert parsed["b"] == "value"
+    assert parsed["c"] == 3
+


src/specify_cli/__init__.py

+             if verbose:
+                 console.print(f"[yellow]Warning: Existing JSON in {existing_path.name} is not an object, using template instead[/yellow]")
+             return new_content
+
+    except (FileNotFoundError, json.JSONDecodeError) as e:
        # If file doesn't exist or is invalid, just use new content
+        if verbose and not isinstance(e, FileNotFoundError):
+             console.print(f"[yellow]Warning: Could not parse existing JSON in {existing_path.name} ({e}), using template instead[/yellow]")


src/specify_cli/__init__.py

+             if verbose:
+                 console.print(f"[yellow]Warning: Existing JSON in {existing_path.name} is not an object, using template instead[/yellow]")
+             return new_content
+
+    except (FileNotFoundError, json.JSONDecodeError) as e:
        # If file doesn't exist or is invalid, just use new content
+        if verbose and not isinstance(e, FileNotFoundError):
+             console.print(f"[yellow]Warning: Could not parse existing JSON in {existing_path.name} ({e}), using template instead[/yellow]")


src/specify_cli/__init__.py

+    import re
+    # Remove single-line comments
+    content = re.sub(r'//.*$', '', content, flags=re.MULTILINE)
+    # Remove multi-line comments
+    content = re.sub(r'/\*.*?\*/', '', content, flags=re.DOTALL)
+    return content
+


CHANGELOG.md

+- feat(cli): support for JSONC (JSON with comments) in VSCode settings.json merging
+- feat(cli): polite deep merge for JSON files that preserves existing user settings
+- feat(cli): basic validation for JSON object structures during initialization
+- feat(cli): strip_json_comments utility for handling VSCode configuration files


…ommas, fix indentation

RbBtSn0w · 2026-03-16T03:37:48Z

I've addressed the feedback regarding JSONC parsing:

Implemented a state-machine tokenizer for strip_json_comments that correctly handles string literals (e.g., URLs).
Added support for trailing commas in objects and arrays.
Fixed indentation issues in src/specify_cli/__init__.py.
Added new test cases in tests/test_merge.py to cover these scenarios.

Copilot

Pull request overview

This PR updates Specify CLI’s VSCode .vscode/settings.json handling to avoid overwriting user settings by adding JSONC parsing support and a “polite” deep-merge strategy during init/upgrade flows.

Changes:

Added strip_json_comments() to support VSCode-style JSONC (line and block comments) and attempted trailing-comma handling.
Implemented “polite” deep merge logic in merge_json_files() to preserve existing user values while adding missing recommended keys.
Added unit tests for comment stripping and merge behavior; updated CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`src/specify_cli/__init__.py`	Adds JSONC stripping + polite deep merge, and uses it when handling `.vscode/settings.json`.
`tests/test_merge.py`	New tests for JSONC stripping and merge behavior (including invalid/non-object existing JSON).
`CHANGELOG.md`	Documents the new JSONC + polite merge behavior in Unreleased notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/specify_cli/__init__.py


-    def deep_merge(base: dict, update: dict) -> dict:
-        """Recursively merge update dict into base dict."""
+    if not isinstance(new_content, dict):


tests/test_merge.py

+def test_strip_json_comments_basic():
+    jsonc = """
+    {
+        // Single line comment
+        "a": 1,
+        "b": "value", /* Multi-line
+                         comment */
+        "c": 3 // Another one
+    }
+    """
+    stripped = strip_json_comments(jsonc)
+    parsed = json.loads(stripped)
+    assert parsed["a"] == 1
+    assert parsed["b"] == "value"
+    assert parsed["c"] == 3
+
+def test_strip_json_comments_with_urls():
+    # URL in string should NOT be stripped
+    jsonc = """
+    {
+        "url": "https://github.com/spec-kit",
+        "path": "//local/path",
+        "commented": 1 // This is a comment
+    }
+    """
+    stripped = strip_json_comments(jsonc)
+    parsed = json.loads(stripped)
+    assert parsed["url"] == "https://github.com/spec-kit"
+    assert parsed["path"] == "//local/path"
+    assert parsed["commented"] == 1
+
+def test_strip_json_comments_trailing_commas():
+    # Trailing commas in objects and arrays
+    jsonc = """
+    {
+        "array": [1, 2, 3, ],
+        "obj": {
+            "a": 1,
+        },
+    }
+    """
+    stripped = strip_json_comments(jsonc)
+    parsed = json.loads(stripped)
+    assert parsed["array"] == [1, 2, 3]
+    assert parsed["obj"]["a"] == 1


tests/test_merge.py

@@ -0,0 +1,98 @@
+import json
+from pathlib import Path
+import pytest


src/specify_cli/__init__.py

+        if char in ('"', "'") and (i == 0 or content[i-1] != '\\'):
+            if not in_string:
+                in_string = True
+                string_char = char
+            elif char == string_char:
+                in_string = False


src/specify_cli/__init__.py

+    # Handle trailing commas: find a comma followed by closing brace/bracket
+    # This regex handles cases like [1, 2, ] or {"a": 1, }
+    clean_content = re.sub(r',\s*([\]}])', r'\1', clean_content)
+


…prove merge robustness

RbBtSn0w · 2026-03-16T03:44:29Z

Expanded test coverage across multiple dimensions:

String literals: Verified URL-like strings and escaped characters are not corrupted.
Trailing commas: Verified support for trailing commas in nested objects and arrays.
BOM handling: Added support for UTF-8 Byte Order Mark (BOM).
Merge conflicts: Verified polite merging preserves existing settings when types mismatch (e.g., string vs object).
Realistic scenarios: Simulated complex settings.json file merging.
All 9 test cases in tests/test_merge.py are passing.

…C/merge validation

RbBtSn0w · 2026-03-16T03:50:18Z

Addressing final review comments:

Refined Backslash Handling: strip_json_comments now determines whether a quote is escaped by correctly counting consecutive backslashes (odd = escaped, even = terminator). Added test cases for even_backslashes and odd_backslashes.
Improved Validation: merge_json_files now explicitly validates that new_content is a dictionary and returns an empty dictionary with a warning if it is not.
Cleanup: Removed the unused pytest import in tests/test_merge.py.
Refined Test Cases: Updated tests to reflect the new validation logic and cover complex escape sequences.

Copilot

Pull request overview

This PR updates Specify CLI’s VSCode initialization/upgrade flow to avoid overwriting a user’s existing .vscode/settings.json by adding JSONC parsing (comments/trailing commas) and performing a “polite” deep merge that preserves existing user values.

Changes:

Add strip_json_comments() to parse VSCode-style JSONC (comments + trailing commas).
Update settings handling to merge template settings into existing settings without overwriting user values (recursive for dicts).
Add unit tests for JSONC stripping + merge behavior and document the feature in the changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`src/specify_cli/__init__.py`	Adds JSONC stripping + polite deep merge and uses it during `.vscode/settings.json` handling.
`tests/test_merge.py`	Adds tests for comment stripping, deep merge behavior, BOM handling, and error cases.
`CHANGELOG.md`	Notes JSONC support and polite merge behavior.

Comments suppressed due to low confidence (1)

src/specify_cli/init.py:673

Merging .vscode/settings.json rewrites the file via json.dump(...), which will drop any existing comments and original formatting even though JSONC parsing is now supported. If preserving comments is a goal, you’ll need a non-destructive update strategy (e.g., patching the text AST) or at least document/warn that comments will be removed on merge.

        if dest_file.exists():
            merged = merge_json_files(dest_file, new_settings, verbose=verbose and not tracker)
            with open(dest_file, 'w', encoding='utf-8') as f:
                json.dump(merged, f, indent=4)
                f.write('\n')

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/test_merge.py

+    """If for some reason new_content is not a dict, return empty dict as safety."""
+    existing_file = tmp_path / "ok.json"
+    existing_file.write_text('{"a": 1}')
+
+    # In the updated implementation, if new_content is not a dict, we return {}
+    assert merge_json_files(existing_file, ["not", "a", "dict"]) == {}


src/specify_cli/__init__.py

+
+    This implementation handles VSCode settings.json files by:
+    1. Skipping comments inside "string literals"
+    2. Correctlly handling escaped quotes by counting backslashes


src/specify_cli/__init__.py

+    clean_content = "".join(result)
+
+    # Handle trailing commas: find a comma followed by closing brace/bracket
+    # This is now safer as we've already stripped comments and identified strings
+    # However, to be 100% safe against commas in strings, we should have handled this 
+    # during the character loop. Let's refine the regex to be very specific to 
+    # JSON structure (comma followed by whitespace and closing char).
+    # Since we've already joined the result, we can apply a final cleanup.
+    # To truly avoid commas in strings, we'd need to mark string ranges.
+    # For now, this regex is standard for basic JSONC to JSON conversion.
+    clean_content = re.sub(r',\s*([\]}])', r'\1', clean_content)
+


src/specify_cli/__init__.py

+        if verbose:
+            console.print(f"[yellow]Warning: Template content for {existing_path.name} is not a dictionary. Skipping merge.[/yellow]")
+        return {}
+


src/specify_cli/__init__.py

+                    console.print(f"[yellow]Warning: Existing JSON in {existing_path.name} is not an object, using template instead[/yellow]")
+                return new_content


… fallback logic

RbBtSn0w · 2026-03-16T04:00:38Z

Final updates to address all Copilot and Maintainer concerns:

String-Safe Trailing Commas: Integrated trailing comma removal into the state-machine loop of strip_json_comments. This ensures commas inside strings (e.g. "val, ]") are strictly preserved.
Secure Fallback Mechanism: Updated merge_json_files to return the existing configuration instead of an empty dictionary when the template (new_content) is invalid. This prevents accidental data loss.
Typo Fix: Corrected "Correctlly" to "Correctly".
Robustness: Added a specific test case test_strip_json_comments_safe_commas_in_strings to verify the state-machine's precision.
Indentation: Normalized all indentations to 4-space multiples.

feat(cli): implement polite deep merge for settings.json and support …

2776f44

…JSONC

RbBtSn0w requested a review from mnriem as a code owner March 16, 2026 03:31

Copilot AI review requested due to automatic review settings March 16, 2026 03:31

Copilot started reviewing on behalf of RbBtSn0w March 16, 2026 03:31 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

refactor(cli): improve JSONC parsing to handle strings and trailing c…

eaf7fb2

…ommas, fix indentation

RbBtSn0w requested a review from Copilot March 16, 2026 03:39

Copilot started reviewing on behalf of RbBtSn0w March 16, 2026 03:40 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

test(cli): add multi-dimensional tests for JSONC and BOM handling, im…

f28e9c1

…prove merge robustness

fix(cli): implement robust backslash escape counting and improve JSON…

845b9f3

…C/merge validation

Copilot AI review requested due to automatic review settings March 16, 2026 03:50

Copilot started reviewing on behalf of RbBtSn0w March 16, 2026 03:50 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

fix(cli): finalize JSONC parsing with safe trailing commas and secure…

67f56c8

… fallback logic

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): polite deep merge for settings.json and support JSONC#1863

feat(cli): polite deep merge for settings.json and support JSONC#1863
RbBtSn0w wants to merge 5 commits intogithub:mainfrom
RbBtSn0w:fix/settings-json-polite-merge

RbBtSn0w commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		console.print(f"[yellow]Warning: Existing JSON in {existing_path.name} is not an object, using template instead[/yellow]")
		return new_content

Conversation

RbBtSn0w commented Mar 16, 2026

Description

Key Changes

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

RbBtSn0w commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants