[ajitpratap0]

Description

Follow-up from #825. Dialect lists had drifted:

• CLI help listed 8 dialects, while 11 are accepted.
• The CLI validation error omitted generic and clickhouse.
• The VS Code gosqlx.dialect enum offered 6 values.
• The GitHub Action accepted 5.
• The MCP enum listed 7.
• The WASM bridge silently ignored bigquery and redshift.

Changes Made

• New keywords.DialectNames(): the single source of truth, in the same order as AllDialects.
• These now derive from it:
  • CLI --dialect help on every command, and validateDialectName errors;
  • parser validation errors;
  • the MCP validate_sql enum;
  • config validation;
  • the WASM bridge.
• These now list all 11 names: GitHub Action input and validation regex (action.yml, action/scripts/validate.sh), the VS Code enum and settings validation, example configs, READMEs, and the CLI, configuration and MCP guides.
• transpile --from/--to deliberately keeps its own list. It supports 8 dialects plus the postgres and mssql aliases; a comment now says so.
• Drift tests: the VS Code package.json enum must match DialectNames(), every --dialect help must name every dialect, and docs or config lines listing dialects must be complete.

Type of Change

• Bug fix (non-breaking). Everything accepted before is still accepted; some surfaces now accept more.

Testing

• task check, task test:cbinding, VS Code extension compile
• go-reviewer pass. Both findings fixed: missed doc and config sites, and the transpile list is now documented.

Breaking Changes

None.

Note: this touches wasm/internal/bridge and pkg/lsp/doc.go, which #825 and the LSP dialect PR also touch. Whichever merges later needs a trivial rebase.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KTj1PEbxzVCETkeYrZKcrE

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 21, 2026 8:50am UTC

[github-actions]

Claude finished @ajitpratap0's task in 20s —— View job

---

Review of PR #829: derive dialect lists from keywords.DialectNames()

• Read the diff and the surrounding code
• Post the review

I read the Go, action, VS Code and WASM changes. I didn't run the tests, so nothing below is verified by a test run.

Summary: This is a clean, low-risk fix. DialectNames() is a good single source of truth. It returns a fresh slice each call, and its test checks order, uniqueness and IsValidDialect agreement. The call sites are simpler than before: the hand-rolled loops and maps in validate.go, config.go and bridge.go are gone. The change also fixes real drift:

• pkg/config and the CLI config rejected generic or clickhouse.
• The WASM bridge silently ignored bigquery and redshift.

Things worth checking (none blocking):

1. Case sensitivity.

   • keywords.IsValidDialect is case-sensitive, and your new comment in cmd_utils.go says so.
   • bridge.Dialect("PostgreSQL") resolves to the default, as it did under the old map. That's consistent, but the old config validators were also case-sensitive, so there's no regression.

2. Action regex duplication.

   • The dialect regex is hard-coded in both action.yml and action/scripts/validate.sh.
   • The drift test covers docs and config lines, but I couldn't confirm it covers these two regexes.
   • If it doesn't, add them, because the action is the surface that drifted before.

3. VS Code alias removal.

   • Dropping 'mariadb': 'mysql' from findClosestDialect is correct now that mariadb is a real value.
   • Other typo suggestions, such as mssql and postgres, still map correctly.

4. Dependency direction.

   • pkg/config and cmd/gosqlx/internal/config now import pkg/sql/keywords.
   • keywords has no dependencies, so there's no cycle. The CLAUDE.md module-dependency section doesn't list config, so no update is needed.

5. transpile list. Keeping its own 8-dialect list plus aliases, with an explanatory comment, is the right call.

6. MCP. mcp.Enum(sqlkeywords.DialectNames()...) is evaluated once at registration. That's fine because the list is static, and the new server test covers it.

7. Rebase risk. As you noted, wasm/internal/bridge and pkg/lsp/doc.go overlap with feat(highlight): dialect-aware SQL syntax highlighting (API, CLI, LSP, WASM, playground) #825 and the LSP dialect PR, so whichever merges later needs a trivial rebase.

Test coverage: Good. The DialectNames unit test, the config tests for clickhouse and generic, the bridge round-trip over every name, the MCP enum test, and the cross-surface dialect_sync_test.go all match the stated goal.

Verdict: LGTM. The one thing I'd double-check is item 2.
· branch fix/dialect-lists

[github-actions]

Claude finished @ajitpratap0's task in 16s —— View job

---

Review of PR #829: derive dialect lists from keywords.DialectNames()

I read the diff and checked the action regexes against the drift test. I didn't run the tests.

Summary: This is a clean, low-risk fix. DialectNames() is a good single source of truth, and the call sites are simpler than before. The change also fixes real drift: config validation rejecting generic and clickhouse, and the WASM bridge silently ignoring bigquery and redshift.

One gap: the Action regexes aren't covered by the drift test.

• action.yml:214 and action/scripts/validate.sh:41 both hard-code the 11-name regex. They're correct today.
• dialect_sync_test.go checks only the VS Code enum, --dialect help, the doc/config lines matched by `?postgresql`?, `?mysql`?, and VALID_DIALECTS. I found no check on either regex. Since the Action is a surface that drifted before, add a small test that extracts the ^(...)$ alternation from both files and compares it to DialectNames(). The action.yml input description at line 51 could be checked the same way.

Other notes (none blocking):

• Case sensitivity: IsValidDialect is case-sensitive, so bridge.Dialect("PostgreSQL") falls back to the default. That matches the old behavior, so there's no regression.
• VS Code: Dropping the mariadb to mysql typo alias in findClosestDialect is correct now that mariadb is a real value.
• Dependencies: config importing keywords creates no cycle, because keywords has no dependencies.
• transpile: Keeping its own list, with the explanatory comment, is the right call.
• MCP: mcp.Enum(DialectNames()...) is evaluated once at registration. That's fine because the list is static, and the new server test covers it.
• Rebase: wasm/internal/bridge and pkg/lsp/doc.go overlap with feat(highlight): dialect-aware SQL syntax highlighting (API, CLI, LSP, WASM, playground) #825 and the LSP dialect PR, so whichever merges later needs a small rebase.

Test coverage: Good. It includes the DialectNames unit test, config tests for clickhouse and generic, the bridge round-trip over every name, the MCP enum test, and the cross-surface sync test.

Verdict: LGTM. I'd add the Action regex check before merging.
· branch fix/dialect-lists