-
Notifications
You must be signed in to change notification settings - Fork 598
Add Move to Schema command for database project files #22705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5332277
7eaaf55
fd01661
4469b13
59b0fb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,11 @@ interface SchemaFolderMovePlan { | |
| newAbsUri: vscode.Uri; | ||
| } | ||
|
|
||
| interface MoveToSchemaSqlToken { | ||
| text: string; | ||
| start: number; | ||
| } | ||
|
|
||
| /** | ||
| * Surfaces a "Move to Schema..." action under the editor's **Refactor...** menu for SQL files in a | ||
| * SQL project. | ||
|
|
@@ -96,8 +101,12 @@ export class SqlMoveToSchemaProvider implements vscode.CodeActionProvider { | |
| }), | ||
| vscode.commands.registerCommand( | ||
| cmdMoveToSchema, | ||
| (document: vscode.TextDocument, position: vscode.Position) => | ||
| provider.runMoveToSchema(document, position), | ||
| (documentOrPath: vscode.TextDocument | string, position?: vscode.Position) => | ||
| typeof documentOrPath === "string" | ||
| ? provider.runMoveToSchemaFromFilePath(documentOrPath) | ||
| : position | ||
| ? provider.runMoveToSchema(documentOrPath, position) | ||
| : undefined, | ||
| ), | ||
| ]; | ||
| } | ||
|
|
@@ -189,6 +198,120 @@ export class SqlMoveToSchemaProvider implements vscode.CodeActionProvider { | |
| await this.applyMove(document, position, selected.label, schemas); | ||
| } | ||
|
|
||
| //#region Tree Entry Local Parsing (replaceable) | ||
| // NOTE: This region exists only to bridge the tree-entry (file path only) flow into the | ||
| // existing Move-to-Schema document+position flow. If STS exposes an API that resolves the | ||
| // movable object position from a file path, replace this entire region with that STS call. | ||
| // TODO Task: https://github.com/microsoft/vscode-mssql/issues/22709 | ||
| /** | ||
| * Starts Move to Schema from a Database Projects tree file path. | ||
| */ | ||
| public async runMoveToSchemaFromFilePath(filePath: string): Promise<void> { | ||
| try { | ||
| const document = await vscode.workspace.openTextDocument(vscode.Uri.file(filePath)); | ||
| const position = this.findMoveToSchemaSymbolPosition(document); | ||
| if (!position) { | ||
| void vscode.window.showInformationMessage(loc.noMovableSymbolAtCursor); | ||
| return; | ||
| } | ||
|
|
||
| await this.runMoveToSchema(document, position); | ||
| } catch (err) { | ||
| void vscode.window.showErrorMessage( | ||
| loc.moveToSchemaRequestFailed(err instanceof Error ? err.message : String(err)), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Finds the position of the object identifier used as the Move-to-Schema cursor target. | ||
| * Supports CREATE/ALTER forms, including CREATE OR ALTER, and schema-qualified names. | ||
| */ | ||
| private findMoveToSchemaSymbolPosition( | ||
| document: vscode.TextDocument, | ||
| ): vscode.Position | undefined { | ||
| const tokens = this.tokenizeMoveToSchemaSql(document.getText()); | ||
| const objectTypes = new Set([ | ||
| "table", | ||
| "view", | ||
| "proc", | ||
| "procedure", | ||
| "function", | ||
| "trigger", | ||
| "sequence", | ||
| ]); | ||
|
|
||
| for (let i = 0; i < tokens.length; i++) { | ||
| const statement = tokens[i].text.toLowerCase(); | ||
| if (statement !== "create" && statement !== "alter") { | ||
| continue; | ||
| } | ||
|
|
||
| let objectTypeIndex = i + 1; | ||
| if ( | ||
| tokens[objectTypeIndex]?.text.toLowerCase() === "or" && | ||
| tokens[objectTypeIndex + 1]?.text.toLowerCase() === "alter" | ||
| ) { | ||
| objectTypeIndex += 2; | ||
| } | ||
|
|
||
| const objectType = tokens[objectTypeIndex]?.text.toLowerCase(); | ||
| if (!objectType || !objectTypes.has(objectType)) { | ||
| continue; | ||
| } | ||
|
|
||
| const firstNameToken = tokens[objectTypeIndex + 1]; | ||
| if (!firstNameToken) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // schema.object -> target object token; object -> target first name token | ||
| if (tokens[objectTypeIndex + 2]?.text === "." && tokens[objectTypeIndex + 3]) { | ||
| return document.positionAt(tokens[objectTypeIndex + 3].start); | ||
| } | ||
| return document.positionAt(firstNameToken.start); | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Tokenizes SQL text into identifiers and separators while skipping whitespace, comments, | ||
| * and string literals so statement detection ignores non-executable text. | ||
| */ | ||
| private tokenizeMoveToSchemaSql(text: string): MoveToSchemaSqlToken[] { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see any unit tests that cover this method. Please add several, especially since we're (temporarily) relying on a complicated REGEX.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, Added the test coverage for the regex method and also created a task for the scriptDom replacement and added as a todo comment in the code. |
||
| // Match only syntax that affects CREATE/ALTER object detection: whitespace and | ||
| // comments/strings to skip, quoted or bare identifiers to keep, and dots for | ||
| // schema-qualified names. The sticky flag keeps token positions exact while the | ||
| // loop below advances over punctuation this parser does not care about. | ||
| const tokenPattern = | ||
| /\s+|--[^\r\n]*|\/\*[\s\S]*?\*\/|'(?:''|[^'])*'|\[(?:[^\]]|\]\])+\]|"(?:""|[^"])*"|`(?:``|[^`])*`|[A-Za-z0-9_$#@]+|\./y; | ||
|
ssreerama marked this conversation as resolved.
|
||
| const tokens: MoveToSchemaSqlToken[] = []; | ||
|
|
||
| for (let index = 0; index < text.length; ) { | ||
| tokenPattern.lastIndex = index; | ||
| const match = tokenPattern.exec(text); | ||
| if (!match) { | ||
| index++; | ||
| continue; | ||
| } | ||
|
|
||
| const value = match[0]; | ||
| index = tokenPattern.lastIndex; | ||
| if (!value || /^\s+$/.test(value)) { | ||
| continue; | ||
| } | ||
| if (value.startsWith("--") || value.startsWith("/*") || value.startsWith("'")) { | ||
| continue; | ||
| } | ||
| tokens.push({ text: value, start: match.index }); | ||
| } | ||
|
|
||
| return tokens; | ||
| } | ||
|
|
||
| //#endregion | ||
|
|
||
| /** | ||
| * Runs the move end to end: asks STS for the script edits, applies them through VS Code's | ||
| * refactor preview, then relocates the definition file and updates the `.sqlproj`. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the whole region is replaceable if STS can give the sql object name from the parser, which script Dom already have. But, needed STS changes which can be done in another release cycle not at the ask mode time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just to provide a quality-of-life improvement, correct? Might be better just to hold this fix back from this release so that we can do it correctly (using STS/ScriptDom)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a chat with Drew and this can be moved to other cycle as is not very urgent one.
have moved the issue to next cycle, updating the PR to draft (will close it and open a new one if required)