You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(editor): open the type list on a PostgreSQL cast with nothing typed - #2925
Found while investigating #2915. Two halves of one mismatch: the docs and the code disagreed about which cursor positions open the popup with nothing typed, and in one of them the code was wrong.
The code half
SQLCompletionService.isSuppressedEmptyPrefix holds the list of clauses that auto-open on an empty prefix. Everything outside it falls to default: return true and waits for Ctrl+Space. That list was written for 0.21.0, which suppressed noisy empty-prefix suggestions in clauses like WHERE.
0.65.0 (#2095) added :: cast completion and registered : as a trigger character, but never added .castTarget to the list. So typing SELECT id:: fired the trigger, produced the type list, and then threw it away. SELECT id::js worked, because a non-empty prefix skips the check entirely.
The type list after :: is exactly what the list is for: short, closed, and the only thing that can go there. .castTarget is now on it.
The other positions the docs claimed stay suppressed, because they are the noise 0.21.0 removed: a bare WHERE, a function's open paren, and a comparison's value side all sit in front of the full column list.
The docs half
docs/features/autocomplete.mdx showed four positions opening on an empty cursor that do not:
SELECT * FROM users WHERE | under Column names
SELECT COUNT(| and WHERE date_column > | under Functions and operators
WHERE status = | under Casts and enum values
Each comment now says Ctrl+Space for …, and the opening paragraph names the positions that do auto-open rather than describing them as "clauses with a short answer", which was too vague to check. SELECT id::| needed no edit: the code change makes it true.
Still open
WHERE status = on an enum column is the one case where the docs' original promise looks right and the code does not deliver it. SQLContext.comparisonColumn is computed for exactly that position and isSuppressedEmptyPrefix never reads it. Allow-listing it is not a one-word change: the suppression runs on the clause, not on what came back, so .where_ would open the whole column list on every comparison rather than the handful of enum labels. Left alone, documented as Ctrl+Space.
castTargetOpensOnAnEmptyPrefix was run against the unchanged switch first and failed; the other five in the new suite pass either way, which is what makes them a pin on the behaviour this does not change.
verify.sh docs: house style and source claims both agree.
swiftlint --strict on both changed Swift files: clean.
Codex review found two real problems and both are fixed in d14a0bc71.
SQL Server also spells scope resolution ::.SQLContextAnalyzer.endsWithCastOperator reads any :: as a cast without consulting the dialect, so unsuppressing .castTarget for everyone answered hierarchyid:: and geometry::STGeomFromText with the MSSQL data-type list, selectable with Return. The gate is now the dialect's own declaration: SQLCompletionService unsuppresses the empty prefix only when the connection's SQLDialectDescriptor declares :: with category .cast, which only PostgreSQL does (PostgreSQLDialect.swift:295). doubleColonWithoutACastOperatorStaysSuppressed covers the MSSQL shape. The mis-classification itself predates this PR and is unchanged: a typed prefix such as hierarchyid::Pa has always reached .castTarget.
SELECT COUNT(| does auto-open, and my docs edit said it did not.( is a trigger character and the analyzer classifies that position as .select, which an empty prefix does not suppress (SQLContextAnalyzerTests.testFunctionArgClause pins it). That line is back to -- Columns and *, and the opening paragraph no longer claims function calls are suppressed; it now lists the clauses that open and names WHERE, HAVING, GROUP BY and ORDER BY as the ones that do not.
379 cases across 7 suites pass, and verify.sh docs agrees on both checks.
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
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.
Found while investigating #2915. Two halves of one mismatch: the docs and the code disagreed about which cursor positions open the popup with nothing typed, and in one of them the code was wrong.
The code half
SQLCompletionService.isSuppressedEmptyPrefixholds the list of clauses that auto-open on an empty prefix. Everything outside it falls todefault: return trueand waits forCtrl+Space. That list was written for 0.21.0, which suppressed noisy empty-prefix suggestions in clauses like WHERE.0.65.0 (#2095) added
::cast completion and registered:as a trigger character, but never added.castTargetto the list. So typingSELECT id::fired the trigger, produced the type list, and then threw it away.SELECT id::jsworked, because a non-empty prefix skips the check entirely.The type list after
::is exactly what the list is for: short, closed, and the only thing that can go there..castTargetis now on it.The other positions the docs claimed stay suppressed, because they are the noise 0.21.0 removed: a bare WHERE, a function's open paren, and a comparison's value side all sit in front of the full column list.
The docs half
docs/features/autocomplete.mdxshowed four positions opening on an empty cursor that do not:SELECT * FROM users WHERE |under Column namesSELECT COUNT(|andWHERE date_column > |under Functions and operatorsWHERE status = |under Casts and enum valuesEach comment now says
Ctrl+Space for …, and the opening paragraph names the positions that do auto-open rather than describing them as "clauses with a short answer", which was too vague to check.SELECT id::|needed no edit: the code change makes it true.Still open
WHERE status =on an enum column is the one case where the docs' original promise looks right and the code does not deliver it.SQLContext.comparisonColumnis computed for exactly that position andisSuppressedEmptyPrefixnever reads it. Allow-listing it is not a one-word change: the suppression runs on the clause, not on what came back, so.where_would open the whole column list on every comparison rather than the handful of enum labels. Left alone, documented asCtrl+Space.Verified
TableProTests, 7 autocomplete suites: 378 cases, 378 passed.castTargetOpensOnAnEmptyPrefixwas run against the unchanged switch first and failed; the other five in the new suite pass either way, which is what makes them a pin on the behaviour this does not change.verify.sh docs: house style and source claims both agree.swiftlint --stricton both changed Swift files: clean.