feat(editor): match SQL autocomplete case to the typed prefix - #2902
Merged
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completing
selnow insertsselect, andSELinsertsSELECT, in the popup label as well as the inserted text. Closes #2833.Root cause
Case was decided at item construction, by a factory that cannot know what the user typed and is shared by vocabularies whose spelling is fixed.
SQLCompletionItem.keyword()wrotekeyword.uppercased()into bothlabelandinsertText, on a struct built ingetCandidateslong before ranking, and the accept path faithfully inserted that constant.That is two defects at once: the case was decided too early, and it was decided on the wrong axis.
kind == .keywordis not a usable discriminator, because it also carries value literals ('active'),*andusers.*, plugin statement completions, and MongoDB's$matchvocabulary. Re-casing by kind would rewrite user data.The second defect already shipped: MongoDB autocomplete offered
$MATCHandDB, and the server rejects both.The fix
The missing concept is that the vocabulary declares whether its spelling is fixed.
SQLCompletionItemgainscaseFolding, defaulting to.fixed. Only SQL keywords, generic built-in functions, operators and DDL type names opt into.caseInsensitive.SQLCompletionCasingis a pure transform applied at one chokepoint,CompletionEngine, which every SQL completion item passes through on its way to a UI surface. That covers the editor popup, the filter panel's Raw SQL field and the compare scope editor in one place. The session's candidate pool stays canonical, so a longer prefix folds from the vocabulary's own spelling.SQLKeywordCasereplaces theuppercaseKeywordsboolean and drives the popup label, the inserted text, the as-you-type rewriter andFormat SQL, so those three can no longer disagree.Behaviour
selselectSELSELECTSelSELECTsElEselectcoucount(), caret between the parensgroup bgroup byThe first cased character decides and nothing else. This is psql's rule:
pg_strdup_keyword_case()branches onislower(ref[0])alone, with no per-character mirroring and no Capitalised arm. Vim'sinfercaseand Emacs'dabbrevagree that one leading capital is not all-caps intent.The four setting values are psql's
COMP_KEYWORD_CASE. The twoMatch what I typevalues differ only in the case they fall back to when the prefix has no cased character, which happens whenever the popup opens on an empty token. The default,Match what I type, otherwise UPPERCASE, reproduces the shipped output for every case that has no typed signal.Identifiers are never re-cased. DBeaver's
Insert casedoes re-case them and lower-cased a real table name out of existence (dbeaver/dbeaver#12054); psql'sidentifier_needs_quotes()excludes catalog identifiers from the keyword-case path for the same reason. #2527 keeps its own per-connection identifier setting and its quoting rule.Why not decide at accept time
SQLCompletionInsertion.resolveis the cheaper place, but it runs only on accept, after the popup has already drawn the label, and the filter panel never called it at all. The request names both the label and the inserted text.Included because the primary fix is not safe or complete without them
MongoDB inserted
$MATCH.MongoCompletionServicerouted$match,$group,dband every collection method through the uppercasing factory. Its whole vocabulary is JavaScript and case-significant, so it is marked.fixedexplicitly rather than inheriting a default.ClickHouse offered 18 function names the server rejects. Read from the server's own catalog on 26.9.1.52:
SELECT name, case_insensitive FROM system.functions. 18 of the 36 names the dialect declared arecase_insensitive = 0, soTOSTRING,UNIQ,MULTIIF,ARRAYJOIN,TOPK,TRIM,LTRIM,RTRIM,MATCH,QUANTILEand the rest were completions that produceCode: 46 UNKNOWN_FUNCTION. Corrected to the catalog spellings.SQLDialectDescriptor.functionNamesAreCaseInsensitiveis what stops the new transform re-casing them into a different wrong spelling; ClickHouse is the only dialect that sets it false, and the generic pool stays case-insensitive there becausecount,if,now,cast,concat,lowerandsubstringare allcase_insensitive = 1.Completion duplicated text after a non-ASCII prefix.
SQLTokenBoundary.isIdentifierCharaccepted onlyA-Z a-z 0-9 _, and it defines the typed prefix both for the analyzer and for the accept path's replacement range. TypingSELECT 名and accepting名前gaveSELECT 名名前. The backward walk now steps by composed character sequence, so surrogate pairs and combining marks are consumed whole.$is deliberately still excluded, because MongoDB's analyzer needs a stage token to start there. ASCII behaviour is byte-identical.The formatter had no lowercase mode.
SQLFormatterOptions.uppercaseKeywordswas a boolean whose false branch kept the spelling already in the statement, except inhandleJoinPrefix, which lowercased. WiringKeyword caseto it would have madeFormat SQLclaim a lowercase it does not produce. It is now a three-stateSQLFormatterKeywordCase, andpreservekeeps the old false behaviour for the one caller that used it.The ClickHouse dialect is declared twice. The app's curated pre-load table carried a verbatim copy of the same broken list, and it is what a completion service gets before the lazy driver bundle activates. Nothing at runtime made the two agree, so
ClickHouseDialectParityTestsreads the plugin's source and the curated table and fails when their function vocabularies diverge.The filter panel bypassed the shared insertion rule.
SQLCompletionInsertion.resolvehad exactly one production call site. The filter field spliced raw text and hard-coded the caret after it, so acceptingCOUNTthere left the caret outside the parentheses. Both surfaces now share one rule.PluginKit
SQLDialectDescriptorgains one field, added through a new init overload with the previous designated init marked@_disfavoredOverload, so no already-built plugin loses a symbol.currentPluginKitVersionis already 31 for this cycle (v0.74.0 shipped 30) and every pluginInfo.plistalready stamps 31, so this additive change reuses the pending number: no new bump, nominimumCompatiblePluginKitVersionchange, and no bulk re-release.Not in scope
statementCompletionsstay fixed-spelling for the twelve engines that populate them. Freezing curated per-plugin vocabulary is the safe default; givingCompletionEntryits own folding flag is a separate change.::keep their existing lower-case spelling. A cast and aCREATE TABLEcolumn use two different conventions for one vocabulary, and one setting cannot express both. Changing it would have regressedSELECT id::fromintegertoINTEGERfor the default setting.Before / After
Typing
selin a query tab, driven against a throwaway sandbox on a Debug build. Same frame in both.Before (and today with
Keyword case = UPPERCASE, which reproduces the old behaviour exactly):After (the shipped default,
Match what I type, otherwise UPPERCASE):An uppercase prefix still gets an uppercase keyword, so this follows what you type rather than just lowercasing:
The setting that replaces the old Auto-uppercase keywords toggle:
Verification
All through
.claude/skills/fix-issue/scripts/verify.sh.generatebuildtest(17 suites owning the changed types)lint(every changed path)mainat the same lines, inTableProTests/, which.swiftlint.yml'sincluded:never reachesdocsabivs merge base@_disfavoredOverloadon the previous designated init. The single removed interface line reappears verbatim with only that attribute prepended, so no symbol disappearedplugins(AllPlugins)OracleNIOpackage on a@TaskLocalmacro, with zero non-macro errors. Pre-existing and unrelateduitestXCTest is trying to Enable UI Automation, and times out withTimed out while enabling automation mode.Twice. The new case is written, is not quarantined, and CI runsTableProUITestsshardedThe ClickHouse facts are measured, not inferred:
SELECT name, case_insensitive FROM system.functionsagainst 26.9.1.52, plus direct execution of each spelling.Reviewed by Codex (
review --scope working-tree). It raised two findings, both real and both fixed here: the formatter's missing lowercase mode, and the duplicated ClickHouse dialect in the app's pre-load table.