microsoft / microsoft/go-sqlcmd

vscode: migrate settings.json writer to full hujson AST manipulation

Open
#767 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
595
Forks
91
Avg merge
9h 35m
Merged PRs (30d)
1

Description

Background

PR #688 added sqlcmd open vscode, which writes the user's settings.json to register a connection profile. The original implementation stripped JSONC comments and trailing commas, marshaled fresh JSON, and saved the user's original to a .sqlcmd-backup file. dlevy flagged that this clobbers the backup on every subsequent run (e.g. creating a second container loses the first backup) and silently destroys hand-authored comments.

PR #688 follow-up commit replaced the strip-and-marshal round trip with github.com/tailscale/hujson. We now parse the original AST and apply an RFC 6902 patch that touches only the two keys we own (mssql.connections, mssql.connectionGroups). Comments, trailing commas, and unrelated user keys round-trip untouched on replace operations, and the .sqlcmd-backup file is no longer needed.

Remaining limitation

hujson's Patch "add" operation (used when the document doesn't yet have an mssql.connections key) can blank a comment immediately adjacent to the insertion point. Unrelated keys are preserved; only a comment line at the insertion site is affected. This is a one-time event per settings.json (subsequent runs use the "replace" path, which fully preserves comments).

Additionally, createProfile / updateOrAddProfile in cmd/modern/root/open/vscode.go operate on map[string]interface{} for the connection array itself. This means per-connection comments inside mssql.connections[*] are not preserved when sqlcmd updates an existing profile.

Proposed work

Refactor cmd/modern/root/open/jsonc.go and the profile-construction helpers to operate directly on hujson.Value AST nodes instead of decoded maps. Specifically:

  1. Walk the parsed root *hujson.Object to find mssql.connections and mssql.connectionGroups members, mutating their Value in place rather than going through a JSON patch.
  2. For the array of profiles, locate the existing profile member by id and rewrite its *hujson.Object fields one at a time, leaving member ordering and surrounding extras intact.
  3. When inserting a new profile or a new top-level key, set ObjectMember.Name.BeforeExtra explicitly so the preceding comment isn't absorbed by hujson's leading-comment extraction.

Verification

The round-trip test in cmd/modern/root/open/jsonc_test.go (TestApplyJSONCSettingsUpdates_PreservesComments) should be extended to cover the add-comment-adjacent case, and a new test should verify per-connection comments inside the mssql.connections array survive an update to an existing profile.

References

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with cmd/modern/root/open/jsonc.go and cmd/modern/root/open/vscode.go to trace the current hujson parsing, profile construction, and update paths. Extend cmd/modern/root/open/jsonc_test.go, including TestApplyJSONCSettingsUpdates_PreservesComments, for add-adjacent and per-connection comments. Done means direct hujson AST updates preserve comments, ordering, and unrelated fields for new and existing profiles.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli, testing
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.