Create VSCode extension for AML #333
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request involve updates to various configuration and documentation files across the project. Key modifications include enhancements to Changes
Poem
Warning Rate limit exceeded@loicknuchel has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
loicknuchel
force-pushed
the
vscode-extension
branch
from
d3d4929
to
c1a2e33
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Outside diff range and nitpick comments (38)
extensions/vscode-aml/resources/schema.aml (2)
8-10: Clarify the structure of thesettingsJSON field.The nested fields
themeandlast_pagewithinsettings json nullablemay not be correctly defined. In AML, to define the structure of a JSON field, you might need to use comments or annotations since JSON fields don't enforce a schema by default.Consider documenting the expected structure of the
settingsfield for clarity:settings json nullable + # Expected JSON structure: + # { + # "theme": "light" | "dark", + # "last_page": "string" + # }
22-22: Specify foreign key relationships explicitly forauthorfield.The
authorfield referencesusers(id)but doesn't specify the relationship type. For clarity, you might want to use->to indicate the foreign key relationship, similar to how it's done withcreated_byandupdated_by.Apply this diff to clarify the relationship:
-author int -> users(id) +author int -> users(id)(If the arrow
->is already present and this is a formatting issue, please disregard.)extensions/vscode-aml/esbuild.js (2)
76-78: Review the necessity of redefining 'global'Defining
globalasglobalThismay not be necessary and could lead to unexpected behaviors. Since the code is targeting the browser environment, andglobalThisis already available, overridingglobalmight introduce conflicts.Consider removing the
defineoption forglobal:define: { - global: 'globalThis', },Ensure that any dependencies relying on
globalare appropriately handled or polyfilled.
80-86: Adjust the order of plugins for proper polyfillingThe
NodeGlobalsPolyfillPluginshould be placed before other plugins that might rely on the polyfills it provides. Ensuring it runs early can prevent issues where subsequent plugins or code expect Node.js globals to be available.Apply this diff to reorder the plugins:
plugins: [ + polyfill.NodeGlobalsPolyfillPlugin({ + process: true, + buffer: true, + }), testBundlePlugin, esbuildProblemMatcherPlugin, - polyfill.NodeGlobalsPolyfillPlugin({ - process: true, - buffer: true, - }), ],backend/lib/azimutt_web/templates/website/docs/getting-started.html.heex (1)
3-7: LGTM! Consider adding ARIA attributes for better accessibility.The added section provides a good transition to the main topics and uses semantic HTML with proper class naming.
Consider enhancing accessibility by adding an ARIA role:
- <p class="lead"> + <p class="lead" role="doc-introduction">extensions/vscode-aml/src/web/test/suite/extension.test.ts (2)
8-9: Consider structuring tests by featureThe current test suite is too broad. Consider organizing tests into feature-specific suites for better maintainability.
Example structure:
suite('AML Extension Tests', () => { suite('Syntax Validation', () => { // Syntax related tests }); suite('Command Execution', () => { // Command related tests }); suite('Integration Tests', () => { // Integration with VSCode API tests }); });
1-5: Add type checking for VSCode API usageWhile the imports are correct, consider adding type assertions or checks when using the VSCode API to ensure type safety.
Example:
import * as assert from 'assert'; import * as vscode from 'vscode'; // Add type guard function isTextEditor(editor: unknown): editor is vscode.TextEditor { return editor instanceof vscode.TextEditor; }extensions/vscode-aml/src/web/test/suite/mochaTestRunner.ts (1)
1-7: Consider enhancing type safety and test reporting.A few suggestions to improve the setup:
- Add type information to the import
- Consider using a more informative reporter for better debugging
-// Imports mocha for the browser, defining the `mocha` global. -import 'mocha/mocha'; +import type * as Mocha from 'mocha'; +// Imports mocha for the browser, defining the `mocha` global. +import 'mocha/mocha.js'; mocha.setup({ ui: 'tdd', - reporter: undefined + reporter: 'spec', // or 'html' for browser-friendly output timeout: 60000 // Optional: Add timeout for long-running tests });extensions/vscode-aml/snippets.json (5)
2-6: Consider adding serial/autoincrement option for primary keysThe primary key snippet covers common types (uuid, int, bigint) but could be enhanced by adding serial/autoincrement as an option, which is frequently used in database schemas.
"Primary key": { "prefix": "id", - "body": ["id ${1|uuid,int,bigint|} pk\n"], + "body": ["id ${1|uuid,int,bigint,serial|} pk\n"], "description": "A Primary Key column" }
7-18: Enhance relationship snippets with descriptions and nullable optionThe relationship snippets could be improved by:
- Adding descriptions to help users understand each relationship type
- Including an optional nullable modifier
"Many to one": { "prefix": ["fk", "->"], - "body": ["-> ${1:entity}(${2:attribute})$0"] + "body": ["-> ${1:entity}(${2:attribute})${3| ,nullable |}$0"], + "description": "Creates a many-to-one relationship (foreign key) to another table" }, "One to one": { "prefix": ["fk", "--"], - "body": ["-- ${1:entity}(${2:attribute})$0"] + "body": ["-- ${1:entity}(${2:attribute})${3| ,nullable |}$0"], + "description": "Creates a one-to-one relationship (unique foreign key) to another table" }, "Many to many": { "prefix": ["fk", "<>", "><"], - "body": ["<> ${1:entity}(${2:attribute})$0"] + "body": ["<> ${1:entity}(${2:attribute})${3| ,nullable |}$0"], + "description": "Creates a many-to-many relationship requiring a junction table" }
19-30: Add descriptions and type options for timestamp snippetsThe timestamp snippets could be enhanced with:
- Descriptions explaining their purpose
- Predefined type options (timestamp, timestamptz)
"Created At": { "prefix": "created_at", - "body": ["created_at ${1:timestamp}=`${2:now()}`\n"], + "body": ["created_at ${1|timestamp,timestamptz|}=`${2:now()}`\n"], + "description": "Adds a creation timestamp column with default value" }, "Updated At": { "prefix": "updated_at", - "body": ["updated_at ${1:timestamp}=`${2:now()}`\n"], + "body": ["updated_at ${1|timestamp,timestamptz|}=`${2:now()}`\n"], + "description": "Adds an update timestamp column with default value" }, "Timestamps": { "prefix": "timestamps", - "body": ["created_at ${1:timestamp}=`${2:now()}`", "updated_at ${1:timestamp}=`${2:now()}`\n"], + "body": ["created_at ${1|timestamp,timestamptz|}=`${2:now()}`", "updated_at ${1|timestamp,timestamptz|}=`${2:now()}`\n"], + "description": "Adds both creation and update timestamp columns" }
31-34: Add description and type options for soft delete snippetThe soft delete snippet could be enhanced with:
- Description explaining the soft delete pattern
- Predefined type options
"Deleted At": { "prefix": "deleted_at", - "body": ["deleted_at ${1:timestamp} nullable\n"], + "body": ["deleted_at ${1|timestamp,timestamptz|} nullable\n"], + "description": "Adds a nullable timestamp column for soft deletes" }
35-46: Enhance user tracking snippets with descriptions and configurable table nameThe user tracking snippets could be improved by:
- Adding descriptions to explain the auditing pattern
- Making the users table name configurable through a shared variable
"Created By": { "prefix": "created_by", - "body": ["created_by -> ${1:users}(id)\n"], + "body": ["created_by -> \${1:${TM_SELECTED_TEXT:users}}(id)\n"], + "description": "Adds a foreign key column to track the creating user" }, "Updated By": { "prefix": "updated_by", - "body": ["updated_by -> ${1:users}(id)\n"], + "body": ["updated_by -> \${1:${TM_SELECTED_TEXT:users}}(id)\n"], + "description": "Adds a foreign key column to track the updating user" }, "Deleted By": { "prefix": "deleted_by", - "body": ["deleted_by nullable -> ${1:users}(id)\n"], + "body": ["deleted_by nullable -> \${1:${TM_SELECTED_TEXT:users}}(id)\n"], + "description": "Adds a nullable foreign key column to track the deleting user" }extensions/vscode-aml/resources/schema.sql (2)
6-16: Consider these improvements to the users table
- Consider using UUID instead of integer for the primary key to support distributed systems and prevent ID enumeration.
- Add email format validation using CHECK constraint.
- Consider using JSONB instead of JSON for better performance and indexing capabilities.
Here's the suggested implementation:
CREATE TABLE users ( - id int PRIMARY KEY, + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), name varchar(64) NOT NULL, - email varchar(256) NOT NULL UNIQUE, + email varchar(256) NOT NULL UNIQUE CHECK (email ~* '^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$'), auth auth_kind NOT NULL, - settings json, + settings jsonb, created_at timestamp NOT NULL DEFAULT now(), updated_at timestamp NOT NULL DEFAULT now(), deleted_at timestamp );
46-53: Consider alternatives to polymorphic associationsThe current design using
item_kindanditem_idas a polymorphic reference can't enforce referential integrity and might lead to invalid data.Consider these alternatives:
- Separate junction tables approach (recommended for data integrity):
CREATE TABLE tracking.user_events ( event_id uuid REFERENCES tracking.events(id), user_id int REFERENCES users(id), PRIMARY KEY (event_id) ); CREATE TABLE tracking.post_events ( event_id uuid REFERENCES tracking.events(id), post_id int REFERENCES cms.posts(id), PRIMARY KEY (event_id) ); CREATE TABLE tracking.comment_events ( event_id uuid REFERENCES tracking.events(id), comment_id int REFERENCES cms.comments(id), PRIMARY KEY (event_id) );
- Check constraint approach (if junction tables are not preferred):
ALTER TABLE tracking.events ADD CONSTRAINT valid_item_reference CHECK ( (item_kind = 'users' AND EXISTS (SELECT 1 FROM users WHERE id = item_id)) OR (item_kind = 'posts' AND EXISTS (SELECT 1 FROM cms.posts WHERE id = item_id)) OR (item_kind = 'comments' AND EXISTS (SELECT 1 FROM cms.comments WHERE id = item_id)) OR item_kind IS NULL );extensions/vscode-aml/README.md (6)
3-3: Add alt text to the marketplace badge for accessibility.The marketplace badge image should include alt text to improve accessibility.
-[](https://marketplace.visualstudio.com/items?itemName=azimutt.vscode-aml) +[](https://marketplace.visualstudio.com/items?itemName=azimutt.vscode-aml)🧰 Tools
🪛 Markdownlint (0.35.0)
3-3: null
Images should have alternate text (alt text)(MD045, no-alt-text)
7-7: Add descriptive alt text to the screenshot for accessibility.The screenshot image should include descriptive alt text to improve accessibility and provide context.
- +
23-28: Update API documentation links to VS Code API.The roadmap items reference Monaco Editor API documentation, but since this is a VS Code extension, they should link to the VS Code Extension API documentation instead.
-auto-complete (cf [registerCompletionItemProvider](https://microsoft.github.io/monaco-editor/typedoc/functions/languages.registerCompletionItemProvider.html)) +auto-complete (cf [registerCompletionItemProvider](https://code.visualstudio.com/api/references/vscode-api#languages.registerCompletionItemProvider)) -rename (cf [registerRenameProvider](https://microsoft.github.io/monaco-editor/typedoc/functions/languages.registerRenameProvider.html)) +rename (cf [registerRenameProvider](https://code.visualstudio.com/api/references/vscode-api#languages.registerRenameProvider)) -hover infos (cf [registerHoverProvider](https://microsoft.github.io/monaco-editor/typedoc/functions/languages.registerHoverProvider.html)) +hover infos (cf [registerHoverProvider](https://code.visualstudio.com/api/references/vscode-api#languages.registerHoverProvider)) -go-to-definition (cf [registerDefinitionProvider](https://microsoft.github.io/monaco-editor/typedoc/functions/languages.registerDefinitionProvider.html)) +go-to-definition (cf [registerDefinitionProvider](https://code.visualstudio.com/api/references/vscode-api#languages.registerDefinitionProvider)) -quick-fixes (cf [registerCodeActionProvider](https://microsoft.github.io/monaco-editor/typedoc/functions/languages.registerCodeActionProvider.html)) +quick-fixes (cf [registerCodeActionProvider](https://code.visualstudio.com/api/references/vscode-api#languages.registerCodeActionProvider)) -hints with actions (cf [registerCodeLensProvider](https://microsoft.github.io/monaco-editor/typedoc/functions/languages.registerCodeLensProvider.html)) +hints with actions (cf [registerCodeLensProvider](https://code.visualstudio.com/api/references/vscode-api#languages.registerCodeLensProvider))
42-42: Fix grammatical error.There's a grammatical error in this sentence.
-VS Code language extensions are made of several and quite independent part. +VS Code language extensions are made of several and quite independent parts.
47-53: Consider adding relative links to referenced files.The development section references several files. Consider adding relative links to make it easier for contributors to navigate to these files directly from the README.
-[language-configuration.json](language-configuration.json) +[language-configuration.json](./language-configuration.json) -[syntaxes/aml.tmLanguage.json](syntaxes/aml.tmLanguage.json) +[syntaxes/aml.tmLanguage.json](./syntaxes/aml.tmLanguage.json) -[src/web/extension.ts](src/web/extension.ts) +[src/web/extension.ts](./src/web/extension.ts) -[snippets.json](snippets.json) +[snippets.json](./snippets.json) -[package.json](package.json) +[package.json](./package.json)
64-65: Enhance publication instructions with security considerations.The publication section could benefit from:
- A note about securely handling the Personal Access Token
- More detailed steps for the publication process
- Version management guidelines
Consider expanding this section with more detailed instructions and security best practices for handling the PAT.
extensions/vscode-aml/package.json (1)
60-87: Improve command title consistency and clarity.There are several inconsistencies in the command definitions:
- Inconsistent capitalization: "New database schema" vs "convert JSON to AML"
- The "convert AML to" command title seems incomplete
- Only the preview command has an icon
Consider applying these changes:
{ "command": "aml.new", "category": "AML", - "title": "New database schema" + "title": "New Database Schema", + "icon": "$(new-file)" }, { "command": "aml.fromJson", "category": "AML", - "title": "convert JSON to AML" + "title": "Convert JSON to AML", + "icon": "$(json)" }, { "command": "aml.fromSQL", "category": "AML", - "title": "convert SQL to AML" + "title": "Convert SQL to AML", + "icon": "$(database)" }, { "command": "aml.convert", "category": "AML", - "title": "convert AML to" + "title": "Convert AML to Other Format", + "icon": "$(export)" },.dockerignore (1)
Line range hint
1-109: Consider cleaning up redundant patterns.The file contains many duplicated patterns from different versions, particularly in the Elm-related sections. Consider consolidating them to improve maintainability:
- The multiple version-specific Elm patterns under
backend/_buildcould be simplified to a single pattern- Some nested patterns are already covered by parent directory ignores
Example consolidation:
- backend/_build/dev/rel/azimutt/lib/azimutt-2.0.1670333486/priv/static/elm/**/dist - backend/_build/dev/rel/azimutt/lib/azimutt-2.0.1670334969/priv/static/elm/**/dist - # ... more version-specific patterns + backend/_build/**/priv/static/elm/**/dist + backend/_build/**/priv/static/elm/**/script.js + backend/_build/**/priv/static/elm/**/script.js.mapextensions/vscode-aml/resources/schema.json (2)
3-24: Consider additional constraints and indexing for the users entityThe users entity is well-structured but could benefit from some additional safeguards:
- Add an email format check constraint
- Consider adding a size limit for the settings JSON
- Add an index on deleted_at to optimize soft delete queries
- Consider a default value for the auth field
{ "name": "users", "attrs": [ {"name": "id", "type": "int", "extra": {"autoIncrement": null}}, {"name": "name", "type": "varchar(64)"}, - {"name": "email", "type": "varchar(256)"}, + {"name": "email", "type": "varchar(256)", "extra": {"check": "email ~ '^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}$'"}}, - {"name": "auth", "type": "auth_kind"}, + {"name": "auth", "type": "auth_kind", "default": "password"}, {"name": "settings", "type": "json", "null": true, "attrs": [ {"name": "theme", "type": "theme"}, {"name": "last_page", "type": "string", "doc": "url of the last visited page"} - ]}, + ], "extra": {"check": "jsonb_typeof(settings) = 'object' AND jsonb_array_length(settings) <= 1000"}}, {"name": "created_at", "type": "timestamp", "default": "`now()`"}, {"name": "updated_at", "type": "timestamp", "default": "`now()`"}, {"name": "deleted_at", "type": "timestamp", "null": true} ], "pk": {"attrs": [["id"]]}, "indexes": [ {"attrs": [["email"]], "unique": true}, - {"name": "user_name_idx", "attrs": [["name"]]} + {"name": "user_name_idx", "attrs": [["name"]]}, + {"name": "user_deleted_at_idx", "attrs": [["deleted_at"]]} ], "extra": {"color": "blue", "tags": ["pii"], "alias": "u"} }
90-95: Consider using schema names instead of line numbers in commentsThe line numbers in comments might become incorrect when the schema changes. Consider using schema names directly:
"extra": { "comments": [ - {"line": 15, "comment": "CMS tables"}, - {"line": 39, "comment": "Tracking tables"} + {"schema": "cms", "comment": "CMS tables"}, + {"schema": "tracking", "comment": "Tracking tables"} ] }.pre-commit-config.yaml (2)
Line range hint
1-191: Add pre-commit hook for the VSCode extensionGiven the TypeScript module resolution issues mentioned in the PR, consider adding a pre-commit hook for the VSCode extension to ensure it builds successfully before commits. This would help catch packaging issues early in the development cycle.
Add this hook configuration:
- id: vscode-aml name: 'extensions/vscode-aml' language: system entry: bash -c 'cd extensions/vscode-aml && exec npm run build && exec npm run package' pass_filenames: false files: extensions/vscode-aml/.*\.ts*$
192-193: Document testing strategy for extensionsThe comments indicate that extensions lack tests and builds. Given the current PR's focus on VSCode extension packaging issues, consider:
- Documenting why these extensions don't have tests/builds in pre-commit
- Adding a testing strategy for the VSCode extension to prevent packaging issues
Would you like help creating a testing strategy document for the extensions directory?
backend/lib/azimutt_web/templates/website/docs/create-your-project.html.heex (3)
Line range hint
24-28: Consider using relative URLs for GitHub links.Using absolute GitHub URLs makes the documentation more fragile to repository changes. Consider using environment variables or relative paths for repository links.
Example approach:
-<a href="https://github.com/azimuttapp/azimutt/blob/main/libs/connector-postgres/src/postgres.ts" target="_blank"> +<a href="<%= @github_repo %>/blob/main/libs/connector-postgres/src/postgres.ts" target="_blank">
39-39: Consider enhancing data privacy clarity.While the statement is accurate, it could be more explicit about the client-side processing to build trust.
Suggested enhancement:
-the parsing is made on the frontend and your schema is shown with no business data sent to Azimutt. +the parsing is performed entirely in your browser (client-side) and your schema is shown with no business data ever sent to Azimutt servers.
56-59: Consider adding JSON schema example.While the documentation now better explains the JSON import process, adding a small example of the expected JSON schema format would make it more user-friendly.
Example addition:
You just have to format it according to the JSON Schema below the import section. +For example: +```json +{ + "tables": [{ + "name": "users", + "columns": [ + {"name": "id", "type": "uuid"}, + {"name": "email", "type": "varchar(255)"} + ] + }] +} +```extensions/vscode-aml/src/web/extension.ts (3)
30-30: Consider managing preview panel state locallyUsing a global variable for the preview panel state could lead to issues in larger extensions. Consider encapsulating this within a class or manager.
-let previewPanel: WebviewPanel | undefined = undefined +class PreviewManager { + private panel: WebviewPanel | undefined; + + getPanel(): WebviewPanel | undefined { + return this.panel; + } + + setPanel(panel: WebviewPanel | undefined): void { + this.panel = panel; + } +} +const previewManager = new PreviewManager();
126-157: Enhance preview functionalityThe preview implementation could be improved:
- Add error handling for preview generation failures
- Consider adding syntax highlighting for the preview
- Add loading state while preview is being generated
Would you like help implementing these enhancements to the preview functionality?
197-197: Make the assignment more explicitThe assignment in the while condition was flagged by static analysis. While this is a common pattern, it can be made more explicit:
- while (match = regex.exec(document.getText())) { + while (true) { + match = regex.exec(document.getText()); + if (!match) break;🧰 Tools
🪛 Biome (1.9.4)
[error] 197-197: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
libs/aml/src/extensions/monaco.ts (5)
Line range hint
73-76: Consider extracting regex patterns to a configuration file.The regex patterns are complex and scattered throughout the code. Consider extracting them to a separate configuration file for better maintainability and reusability.
Create a new file
monaco.patterns.ts:export const patterns = { entity: /^[a-zA-Z_][a-zA-Z0-9_#]*/, attributeName: /^ +[a-zA-Z_][a-zA-Z0-9_#]*/, // ... other patterns }
Line range hint
73-74: Add documentation for regex patterns.The regex patterns are crucial for the language support but lack documentation explaining their purpose and expected matches.
Add JSDoc comments:
+/** + * Matches valid entity names in AML. + * Examples: User, Product_Category, Order#1 + */ export const entityRegex = /^[a-zA-Z_][a-zA-Z0-9_#]*/ +/** + * Matches valid attribute names with leading spaces. + * Examples: " name", " created_at", " user_id" + */ export const attributeNameRegex = /^ +[a-zA-Z_][a-zA-Z0-9_#]*/Also applies to: 86-87
Line range hint
4-4: Address TODO comments.There are unimplemented features mentioned in TODO comments that could enhance the editor experience.
Would you like me to help implement:
- Hover provider to show entity/type definition or AML doc
- Refactoring support for renaming entities/attributes
Also applies to: 73-74
Line range hint
301-315: Enhance code actions provider with more quick fixes.The code actions provider currently only handles legacy replacements. Consider adding more quick fixes for common issues:
- Adding missing properties
- Fixing type mismatches
- Adding missing relations
export const codeAction = (opts: {} = {}): CodeActionProvider => ({ provideCodeActions(model: ITextModel, range: Range, context: CodeActionContext, token: CancellationToken): ProviderResult<CodeActionList> { const actions: CodeAction[] = []; if (context.markers.length > 0) { // Handle different marker types context.markers.forEach(marker => { if (marker.message.includes('missing property')) { actions.push(createAddPropertyAction(marker, model, range)); } // Add more handlers }); } return {actions, dispose() {}}; } });
Line range hint
401-421: Add error handling to helper functions.The helper functions like
suggestTextandsuggestSnippetshould validate their inputs to prevent runtime errors.function suggestText(text: string, kind: CompletionItemKind, position: Position, opts: {insert?: string, documentation?: string} = {}): CompletionItem { if (!text || !position) { throw new Error('Invalid arguments: text and position are required'); } return { label: text.trim(), kind, insertText: opts.insert || text, range: { startLineNumber: position.lineNumber, startColumn: position.column, endLineNumber: position.lineNumber, endColumn: position.column, }, documentation: opts.documentation, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (13)
extensions/browser-extension/public/assets/azimutt_128.pngis excluded by!**/*.pngextensions/browser-extension/public/assets/azimutt_16.pngis excluded by!**/*.pngextensions/browser-extension/public/assets/azimutt_32.pngis excluded by!**/*.pngextensions/browser-extension/public/assets/azimutt_48.pngis excluded by!**/*.pngextensions/desktop/images/icon.icois excluded by!**/*.icoextensions/desktop/images/icon.pngis excluded by!**/*.pngextensions/vscode-aml/assets/icon-white.pngis excluded by!**/*.pngextensions/vscode-aml/assets/icon-white.svgis excluded by!**/*.svgextensions/vscode-aml/assets/icon.pngis excluded by!**/*.pngextensions/vscode-aml/assets/icon.svgis excluded by!**/*.svgextensions/vscode-aml/assets/screenshot.pngis excluded by!**/*.pngextensions/vscode-aml/package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
.dockerignore(1 hunks).gitignore(1 hunks).pre-commit-config.yaml(1 hunks)backend/.gitignore(1 hunks)backend/lib/azimutt_web/templates/website/docs/create-your-project.html.heex(4 hunks)backend/lib/azimutt_web/templates/website/docs/getting-started.html.heex(1 hunks)backend/lib/azimutt_web/templates/website/docs/what-is-azimutt.html.heex(3 hunks)extensions/browser-extension/package.json(1 hunks)extensions/desktop/package.json(1 hunks)extensions/vscode-aml/.gitignore(1 hunks)extensions/vscode-aml/.npmrc(1 hunks)extensions/vscode-aml/.vscode/extensions.json(1 hunks)extensions/vscode-aml/.vscode/launch.json(1 hunks)extensions/vscode-aml/.vscode/settings.json(1 hunks)extensions/vscode-aml/.vscode/tasks.json(1 hunks)extensions/vscode-aml/.vscodeignore(1 hunks)extensions/vscode-aml/CHANGELOG.md(1 hunks)extensions/vscode-aml/README.md(1 hunks)extensions/vscode-aml/esbuild.js(1 hunks)extensions/vscode-aml/eslint.config.mjs(1 hunks)extensions/vscode-aml/language-configuration.json(1 hunks)extensions/vscode-aml/package.json(1 hunks)extensions/vscode-aml/resources/schema.aml(1 hunks)extensions/vscode-aml/resources/schema.json(1 hunks)extensions/vscode-aml/resources/schema.sql(1 hunks)extensions/vscode-aml/snippets.json(1 hunks)extensions/vscode-aml/src/web/extension.ts(1 hunks)extensions/vscode-aml/src/web/test/suite/extension.test.ts(1 hunks)extensions/vscode-aml/src/web/test/suite/mochaTestRunner.ts(1 hunks)extensions/vscode-aml/syntaxes/aml.tmLanguage.json(1 hunks)extensions/vscode-aml/tsconfig.json(1 hunks)gateway/README.md(1 hunks)integration/README.md(0 hunks)integration/compose.yaml(0 hunks)integration/mysql.sql(0 hunks)integration/postgres.sql(0 hunks)libs/aml/src/extensions/monaco.ts(1 hunks)libs/connector-bigquery/README.md(1 hunks)libs/connector-couchbase/README.md(1 hunks)libs/connector-mariadb/README.md(1 hunks)libs/connector-mongodb/README.md(1 hunks)libs/connector-mysql/README.md(1 hunks)libs/connector-oracle/README.md(1 hunks)libs/connector-postgres/README.md(1 hunks)libs/connector-snowflake/README.md(1 hunks)libs/connector-sqlserver/README.md(1 hunks)package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (4)
- integration/README.md
- integration/compose.yaml
- integration/mysql.sql
- integration/postgres.sql
✅ Files skipped from review due to trivial changes (24)
- .gitignore
- backend/.gitignore
- extensions/browser-extension/package.json
- extensions/vscode-aml/.gitignore
- extensions/vscode-aml/.npmrc
- extensions/vscode-aml/.vscode/extensions.json
- extensions/vscode-aml/.vscode/launch.json
- extensions/vscode-aml/.vscode/settings.json
- extensions/vscode-aml/.vscode/tasks.json
- extensions/vscode-aml/.vscodeignore
- extensions/vscode-aml/CHANGELOG.md
- extensions/vscode-aml/language-configuration.json
- extensions/vscode-aml/syntaxes/aml.tmLanguage.json
- extensions/vscode-aml/tsconfig.json
- gateway/README.md
- libs/connector-bigquery/README.md
- libs/connector-couchbase/README.md
- libs/connector-mariadb/README.md
- libs/connector-mongodb/README.md
- libs/connector-mysql/README.md
- libs/connector-oracle/README.md
- libs/connector-postgres/README.md
- libs/connector-snowflake/README.md
- libs/connector-sqlserver/README.md
🧰 Additional context used
🪛 Markdownlint (0.35.0)
extensions/vscode-aml/README.md
3-3: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🪛 Biome (1.9.4)
extensions/vscode-aml/src/web/extension.ts
[error] 197-197: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (22)
extensions/vscode-aml/resources/schema.aml (1)
31-31: Ensure consistency in foreign key declarations.
In post_id int -> cms.posts(id), make sure that the foreign key relationship is correctly defined and consistent with other relationships in the schema.
Run the following script to verify foreign key declarations:
✅ Verification successful
Foreign key declaration is consistent with schema design
The foreign key post_id int -> cms.posts(id) is correctly defined and follows the same pattern as other foreign key relationships in the schema:
- It references an existing table
cms.posts - It references the primary key
idof that table - The data type
intmatches the referenced column's type - The syntax is consistent with other foreign keys in the schema (e.g.,
author int -> users(id),created_by -> users(id))
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Ensure all foreign key fields correctly reference existing tables and fields.
# Check for foreign key references
rg -A 0 $'->' *.aml
Length of output: 427
Script:
#!/bin/bash
# Search for foreign key references in the schema file
rg "\->" "extensions/vscode-aml/resources/schema.aml"
# Also check for table definitions to verify they exist
rg "table" "extensions/vscode-aml/resources/schema.aml"
Length of output: 299
Script:
#!/bin/bash
# Let's get more context around these foreign keys and table definitions
rg -B 2 -A 2 "table|schema" "extensions/vscode-aml/resources/schema.aml"
# Also get the full content of the schema file to understand the complete structure
cat "extensions/vscode-aml/resources/schema.aml"
Length of output: 1597
extensions/vscode-aml/esbuild.js (2)
88-94: Handle cleanup in watch mode to prevent resource leaks
When running in watch mode, the esbuild context is not disposed of upon process termination, which could lead to resource leaks. It's good practice to handle termination signals to gracefully dispose of resources.
Consider adding listeners for termination signals to dispose of the esbuild context:
if (watch) {
await ctx.watch();
+ // Handle process termination signals
+ process.on('SIGINT', async () => {
+ await ctx.dispose();
+ process.exit();
+ });
} else {
await ctx.rebuild();
await ctx.dispose();
}Would you like assistance in implementing this cleanup?
48-49:
Ensure glob pattern correctly matches test files
The glob pattern used may not match test files with extensions .test.ts and .test.tsx as intended. The {} braces in the pattern are for brace expansion, but in some glob implementations, this requires enabling the braceExpansion option.
Apply this diff to adjust the glob pattern:
- const files = await glob.glob('*.test.{ts,tsx}', { cwd: testsRoot, posix: true });
+ const files = await glob.glob('*.test.@(ts|tsx)', { cwd: testsRoot, posix: true, braceExpansion: true });Alternatively, ensure that glob is configured to support brace expansion or use a different pattern such as:
+ const files = await glob.glob('*.test.*(ts|tsx)', { cwd: testsRoot, posix: true });Please verify that the glob library version supports the used pattern and options.
pnpm-workspace.yaml (1)
5-6: LGTM! Good structural organization.
Moving extension packages under a dedicated "extensions" directory improves the project's organization and maintainability.
Let's verify that all related configurations have been updated to reflect this structural change:
✅ Verification successful
✅ Directory structure change is properly implemented
The verification confirms that:
- Both directories exist in their new locations:
extensions/browser-extension/andextensions/desktop/ - Each extension has its own package.json file in place
- Root package.json has been updated to handle the new structure (e.g.,
--filter=!\"./extensions/desktop\")
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all configurations are consistent with the new directory structure
# Expected: All references should use the new paths under "extensions/"
# Check package.json files for old paths
echo "Checking package.json files for old paths..."
rg -l "\"(directory|path)\": *\"(browser-extension|desktop)\""
# Check for any remaining references to old paths in various config files
echo "Checking config files for old paths..."
rg -l "browser-extension/|desktop/" \
--type yaml \
--type json \
--type toml \
--glob '!pnpm-workspace.yaml' \
--glob '!**/node_modules/**'
Length of output: 379
Script:
#!/bin/bash
# Let's try a different approach to verify the structural changes
# 1. First, check if the directories exist in the new location
echo "Checking if directories exist in the new structure..."
fd -t d "^(browser-extension|desktop)$" extensions/
# 2. Look for any package.json files in these directories
echo -e "\nChecking package.json files in extension directories..."
fd -t f "package.json" extensions/
# 3. Check for references to these paths in any file (excluding binary and generated files)
echo -e "\nChecking for any remaining references to old paths..."
rg -l "(browser-extension|desktop)" \
--type-not binary \
--type-not lock \
--glob '!**/node_modules/**' \
--glob '!**/.git/**' \
--glob '!pnpm-workspace.yaml'
# 4. Check the root package.json specifically
echo -e "\nChecking root package.json content..."
cat package.json
Length of output: 2374
extensions/vscode-aml/src/web/test/suite/mochaTestRunner.ts (1)
1-26: Consider browser-specific test infrastructure improvements.
As this is a browser-based test runner for a VSCode extension, consider these architectural improvements:
- Add browser compatibility checks
- Implement proper test timeouts and memory management
- Add integration with VSCode's test API
Let's verify the VSCode extension test setup:
extensions/vscode-aml/eslint.config.mjs (1)
1-28: Verify ESLint configuration with TypeScript files.
Let's verify that the ESLint configuration properly handles TypeScript files and module resolution.
✅ Verification successful
ESLint configuration is properly set up for TypeScript
The ESLint configuration is correctly set up with proper TypeScript support:
- TypeScript files are present and follow standard import patterns
tsconfig.jsonhas appropriate module resolution (Node16) and strict type checking- ESLint config properly integrates TypeScript parser and plugin with necessary rules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ESLint configuration with TypeScript files
# Check for TypeScript files in the extension
echo "Checking for TypeScript files..."
fd -e ts -d 2 . extensions/vscode-aml
# Check for existing import statements
echo "Checking import statements..."
rg "import .* from" --type ts extensions/vscode-aml
# Check for tsconfig.json
echo "Checking TypeScript configuration..."
cat extensions/vscode-aml/tsconfig.json
Length of output: 1531
package.json (2)
33-33: LGTM! Path updated to reflect new directory structure.
The build script filter has been correctly updated to maintain the same behavior while reflecting the new directory structure where components are organized under the 'extensions' directory.
33-33: Verify workspace configuration consistency and build requirements.
Since this PR introduces a new VSCode extension, please verify:
- If the VSCode extension should also be excluded from the main build
- If the path patterns are consistent across all workspace configuration files
✅ Verification successful
Let me gather more information about the build configurations and exclusions to make a proper assessment.
Build configuration is correctly set up for extensions
The current workspace configuration is consistent and appropriate:
-
The VSCode extension (vscode-aml) doesn't need to be excluded from the main build because:
- It has its own independent build process using
vscode:prepublishandpackage-webscripts - It's not included in the workspace packages list in
pnpm-workspace.yaml
- It has its own independent build process using
-
The desktop extension exclusion is correct because:
- It uses Electron-specific build tools (
electron-forge make) - It's explicitly listed in the workspace but excluded from the main build command
- This separation prevents conflicts between desktop and web-based builds
- It uses Electron-specific build tools (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for workspace configuration consistency and build patterns
# Check workspace configuration files for path patterns
echo "Checking pnpm-workspace.yaml for extension paths:"
cat pnpm-workspace.yaml
# Look for other build-related configuration files
echo -e "\nChecking for other build configuration files:"
fd -t f "^(package|tsconfig|vite.config|rollup.config|webpack.config).*\.(json|js|ts)$" -x echo "Found config: {}"
# Check if vscode extension is referenced in any build configs
echo -e "\nChecking for vscode extension references:"
rg -l "vscode|vsce|extension" -g "package.json" -g "*.config.*"
Length of output: 3586
Script:
#!/bin/bash
# Check the build scripts and dependencies in package.json files
echo "Checking root package.json build script:"
cat package.json | jq -r '.scripts.build'
echo -e "\nChecking VSCode extension package.json:"
cat extensions/vscode-aml/package.json | jq -r '{scripts,engines,devDependencies}'
echo -e "\nChecking desktop extension package.json:"
cat extensions/desktop/package.json | jq -r '{scripts,build}'
echo -e "\nChecking if other packages have special build configurations:"
for f in $(fd -t f "package.json" -E "node_modules"); do
echo "=== $f ==="
cat "$f" | jq -r 'select(.scripts.build != null) | .scripts.build'
done
Length of output: 8698
extensions/vscode-aml/snippets.json (1)
1-47: Well-structured snippets covering essential database patterns
The snippets provide a comprehensive set of templates for common database patterns including:
- Primary keys
- Various relationship types
- Timestamp tracking
- Soft deletes
- User auditing
The structure is clean and follows VSCode snippet conventions.
extensions/desktop/package.json (2)
20-20: LGTM! Directory path update is consistent with repository restructuring.
The change from "desktop" to "extensions/desktop" aligns with the new project structure.
Let's verify the consistency of this change across the workspace:
✅ Verification successful
Directory path update is consistent with monorepo workspace configuration
The verification confirms that:
- The new path "extensions/desktop" is correctly listed in the workspace packages
- No references to the old "desktop" directory path remain in package.json files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the directory structure changes are consistent across workspace configuration
# Check if the new path exists and is referenced in workspace config
fd -t f "pnpm-workspace.yaml" -x cat {} \; | grep -A 5 "packages:"
# Check if there are any remaining references to the old path
rg "\"directory\":\s*\"desktop\"" -g "package.json"
Length of output: 231
Line range hint 1-67: Consider updating TypeScript configuration to address module resolution issues.
Given the PR objectives mentioning TypeScript module resolution issues, particularly with @azimutt/* workspace dependencies, consider the following recommendations:
- Ensure
tsconfig.jsonis configured with"moduleResolution": "bundler"for better ESM compatibility - Add
.ts,.js,.mts,.mjsextensions to thecompilerOptions.moduleResolutionsettings
Let's check the current TypeScript configuration:
backend/lib/azimutt_web/templates/website/docs/what-is-azimutt.html.heex (4)
5-6: LGTM! Content improvements enhance clarity.
The rephrasing in the lead paragraph improves readability while maintaining a professional tone.
20-20: Verify the Gateway documentation path exists.
The added link improves navigation, but we should ensure the referenced documentation page exists.
✅ Verification successful
Link to Gateway documentation is valid
The gateway documentation file exists at backend/lib/azimutt_web/templates/website/docs/gateway.html.heex, confirming that the added link is correctly pointing to an existing documentation page.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the Gateway documentation page exists
# Expected: Find the gateway documentation file
fd --type f "gateway" backend/lib/azimutt_web/templates/website/docs/
Length of output: 134
34-34: Verify the CLI documentation path exists.
The added link follows the consistent documentation pattern, but we should verify the referenced page exists.
✅ Verification successful
CLI documentation path exists and is correctly referenced
The link to the CLI documentation is valid as the file cli.html.heex exists at the expected location in the documentation directory.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the CLI documentation page exists
# Expected: Find the CLI documentation file
fd --type f "cli" backend/lib/azimutt_web/templates/website/docs/
Length of output: 126
40-40: Verify the converters documentation path exists.
The added link maintains documentation consistency, but we should verify the referenced page exists.
✅ Verification successful
Link to converters documentation is valid
The converters documentation page exists at backend/lib/azimutt_web/templates/website/docs/converters.html.heex, confirming that the added link is correctly pointing to an existing resource.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the converters documentation page exists
# Expected: Find the converters documentation file
fd --type f "converters" backend/lib/azimutt_web/templates/website/docs/
Length of output: 140
extensions/vscode-aml/package.json (1)
9-11: Consider lowering the minimum VSCode version requirement.
The extension requires VSCode ^1.95.0 which is quite recent. Consider if you really need features from this version, as lowering it would increase the potential user base.
.dockerignore (1)
107-109: LGTM! Verify the extension path restructuring.
The new ignore patterns for the browser extension look correct and follow standard practices.
Let's verify the extension path restructuring:
✅ Verification successful
Path restructuring verified and correctly implemented
The extension path restructuring has been successfully verified:
- The
extensions/directory exists and contains the browser extension in the correct location - The browser extension package is properly configured with the new path in
package.json - All references to the browser extension in the workspace configuration (
pnpm-workspace.yaml) use the correct path - No references to old/incorrect paths were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the extension directory structure and package locations
# Check if the extensions directory exists and contains expected subdirectories
fd --type d --max-depth 2 '^extensions$'
# Check for any remaining references to old paths
rg -g '!.dockerignore' 'browser-extension/(?!.*extensions/)'
Length of output: 374
Script:
#!/bin/bash
# Let's try a different approach to verify the extension path restructuring
# Check if extensions directory exists and list its contents
ls -la extensions/
# Find all package.json files under extensions directory
fd package.json extensions/
# Search for any browser-extension related paths in the codebase
rg -g '!.dockerignore' 'browser-extension'
# Look for any extension-related configuration files
fd -e config.js -e manifest.json extensions/
Length of output: 1167
extensions/vscode-aml/resources/schema.json (2)
73-83: LGTM! Relations are well-defined
The relations section properly defines all foreign key relationships, including the polymorphic associations for the events table. The inline attribute is appropriately used.
84-89: LGTM! Types are well-defined
The type definitions are clear and properly namespaced. The values are appropriate for each enum type.
backend/lib/azimutt_web/templates/website/docs/create-your-project.html.heex (1)
48-50: LGTM! Documentation improvements for Prisma section.
The changes enhance clarity by properly describing Prisma and providing relevant documentation links.
extensions/vscode-aml/src/web/extension.ts (1)
43-43: 🛠️ Refactor suggestion
Consider implementing cleanup in deactivate
The empty deactivate function might miss opportunities for proper cleanup. Consider disposing of any remaining resources, especially the preview panel.
-export function deactivate() {}
+export function deactivate() {
+ if (previewPanel) {
+ previewPanel.dispose();
+ previewPanel = undefined;
+ }
+}Likely invalid or redundant comment.
libs/aml/src/extensions/monaco.ts (1)
58-58: Verify the regex pattern change for attribute types.
The regex pattern has been updated to allow one or more spaces around keywords. While this change improves flexibility, we should ensure it doesn't introduce any unintended matches.
✅ Verification successful
Let me gather more specific information about the attribute type usage in the codebase.
This will help us verify if the regex pattern matches the actual usage patterns in the codebase and test files.
Based on the search results, I can now verify the regex pattern change. The codebase shows extensive usage of SQL data types in various formats, including:
- Simple types:
varchar,text,int,numeric, etc. - Types with parameters:
varchar(10),varchar(140), etc. - Types with spaces:
double precision - Types with modifiers:
timestamp with time zone,character varying
The updated regex pattern in the review accommodates these variations by:
- Using
+for spaces to handle types likedouble precisionandtimestamp with time zone - Supporting optional parameters with
\d+ - Including all common SQL data types
- Maintaining word boundaries with
\b
The regex pattern change is correct and matches the actual usage in the codebase.
The pattern matches all attribute type declarations found in the codebase while maintaining proper word boundaries and flexibility for spaces. No issues were found with the updated pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the regex pattern against sample AML files to ensure it matches correctly
# Search for attribute type declarations in AML files
rg -l '\.aml$' | while read -r file; do
echo "=== $file ==="
rg -A 1 '^\s+[a-zA-Z_][a-zA-Z0-9_#]*\s+\w+' "$file"
done
Length of output: 7135
Script:
#!/bin/bash
# Search for actual attribute type declarations in the codebase
rg -A 1 'type\s+(varchar|text|int|numeric|float|double|boolean|timestamp|date|time|interval|json|string|number)' --type ts
# Also check for any test files that might contain attribute type examples
fd -e test.ts -e spec.ts | xargs rg 'type.*:.*\b(varchar|text|int|numeric|float|double|boolean|timestamp|date|time|interval|json|string|number)\b'
Length of output: 69233
| format: 'cjs', | ||
| minify: production, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use ECMAScript module format for browser target
The build configuration specifies format: 'cjs', which outputs CommonJS modules. Since the target platform is the browser, using ECMAScript modules ('esm') is more appropriate and ensures better compatibility with modern browsers.
Apply this diff to change the module format:
format: 'cjs',
+// Use 'esm' format for browser compatibility
-format: 'cjs',
+format: 'esm',Committable suggestion skipped: line range outside the PR's diff.
backend/lib/azimutt_web/templates/website/docs/create-your-project.html.heex
Outdated
Show resolved
Hide resolved
| class AmlDocumentSymbolProvider implements DocumentSymbolProvider { | ||
| provideDocumentSymbols(document: TextDocument, token: CancellationToken): ProviderResult<SymbolInformation[] | DocumentSymbol[]> { | ||
| const symbols: DocumentSymbol[] = [] | ||
| const regex = /(^|\n)(type\s+)?((?:[a-zA-Z_][a-zA-Z0-9_]*\.)?[a-zA-Z_][a-zA-Z0-9_]*)/g | ||
| let match: RegExpExecArray | null = null | ||
| while (match = regex.exec(document.getText())) { | ||
| const [all, lr, keyword, name] = match || [] | ||
| if (name === 'rel') { continue } | ||
| const range = new Range( | ||
| document.positionAt(match.index + lr.length + (keyword || '').length), | ||
| document.positionAt(match.index + all.length) | ||
| ) | ||
| // see https://microsoft.github.io/monaco-editor/typedoc/interfaces/languages.DocumentSymbol.html | ||
| symbols.push(new DocumentSymbol( | ||
| name, | ||
| ``, // TODO: set entity doc if available | ||
| keyword?.trim() === 'type' ? SymbolKind.Enum : SymbolKind.Class, | ||
| range, | ||
| range | ||
| )) | ||
| } | ||
| return symbols | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using proper parsing instead of regex
The current regex-based symbol extraction might be fragile for complex AML syntax. Consider:
- Using a proper parser for more reliable symbol extraction
- Adding support for nested symbols
- Including more symbol information (fields, relationships)
Example enhancement:
class AmlDocumentSymbolProvider implements DocumentSymbolProvider {
provideDocumentSymbols(document: TextDocument, token: CancellationToken): ProviderResult<SymbolInformation[] | DocumentSymbol[]> {
- const regex = /(^|\n)(type\s+)?((?:[a-zA-Z_][a-zA-Z0-9_]*\.)?[a-zA-Z_][a-zA-Z0-9_]*)/g;
+ // TODO: Use proper AML parser when @azimutt/aml is available
+ const symbols: DocumentSymbol[] = [];
+ const lines = document.getText().split('\n');
+ let currentTable: DocumentSymbol | undefined;
+
+ for (let i = 0; i < lines.length; i++) {
+ const line = lines[i].trim();
+ if (line && !line.startsWith('#')) {
+ if (!line.startsWith(' ')) {
+ // Table definition
+ const range = new Range(i, 0, i, line.length);
+ currentTable = new DocumentSymbol(
+ line,
+ '',
+ SymbolKind.Class,
+ range,
+ range
+ );
+ symbols.push(currentTable);
+ } else if (currentTable) {
+ // Field definition
+ const range = new Range(i, 0, i, line.length);
+ currentTable.children.push(new DocumentSymbol(
+ line.trim(),
+ '',
+ SymbolKind.Field,
+ range,
+ range
+ ));
+ }
+ }
+ }
return symbols;
}
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 197-197: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (8)
extensions/vscode-aml/README.md (5)
3-3: Add alt text to the marketplace badge for accessibility.The marketplace badge image should include descriptive alt text.
-[](https://marketplace.visualstudio.com/items?itemName=azimutt.vscode-aml) +[](https://marketplace.visualstudio.com/items?itemName=azimutt.vscode-aml)🧰 Tools
🪛 Markdownlint (0.35.0)
3-3: null
Images should have alternate text (alt text)(MD045, no-alt-text)
7-7: Use relative path for screenshot image.The absolute path starting with
/might not work correctly in all contexts. Consider using a relative path instead.- +
19-30: Consider creating GitHub issues for roadmap items.The roadmap contains several valuable features. To better track progress and allow community participation, consider creating GitHub issues for each planned feature. This will help with:
- Progress tracking
- Community discussions
- Feature prioritization
Would you like me to help create GitHub issues for these roadmap items?
42-42: Fix grammatical error in sentence.There's a grammatical error in the sentence.
-VS Code language extensions are made of several and quite independent part. +VS Code language extensions are made of several and quite independent parts.🧰 Tools
🪛 LanguageTool
[uncategorized] ~42-~42: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e made of several and quite independent part. For general knowledge, look at the [ex...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
54-59: Consider structuring development tips as a list.The tips section would be more readable as a structured list.
-Tips: - -Debug extension via F5 (Run Web Extension) -Relaunch the extension from the debug toolbar after changing code in `src/web/extension.ts` -Reload (`Ctrl+R` or `Cmd+R` on Mac) the VS Code window with your extension to load your changes +Tips: + +1. Debug extension via F5 (Run Web Extension) +2. Relaunch the extension from the debug toolbar after changing code in `src/web/extension.ts` +3. Reload (`Ctrl+R` or `Cmd+R` on Mac) the VS Code window with your extension to load your changesextensions/vscode-aml/src/web/extension.ts (3)
43-43: Consider implementing cleanup in deactivate()The
deactivatefunction should clean up any resources that were created during the extension's lifetime, such as disposing of thepreviewPanelif it exists.-export function deactivate() {} +export function deactivate() { + if (previewPanel) { + previewPanel.dispose(); + previewPanel = undefined; + } +}
30-30: Consider using a Map for managing multiple preview panelsThe current singleton pattern with
previewPanellimits the extension to a single preview. Consider using a Map to support multiple preview panels, keyed by document URI.-let previewPanel: WebviewPanel | undefined = undefined +const previewPanels = new Map<string, WebviewPanel>(); function previewAml(editor: TextEditor, context: ExtensionContext) { if (editor.document.languageId !== 'aml') { vscode.window.showErrorMessage('Needs AML file to preview it.') return } - if (!previewPanel) { - previewPanel = vscode.window.createWebviewPanel(...) + const uri = editor.document.uri.toString(); + let panel = previewPanels.get(uri); + if (!panel) { + panel = vscode.window.createWebviewPanel(...) + previewPanels.set(uri, panel);Also applies to: 131-156
168-184: Enhance preview HTML with proper styling and error handlingThe current preview HTML is very basic. Consider:
- Adding CSS for better styling
- Implementing proper error handling for invalid AML
- Adding loading states for large files
function buildAmlPreview(aml: string): string | undefined { const content = aml.trim() return `<!DOCTYPE html> <html lang="en"> <head> <meta charset="UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> <title>AML preview</title> + <style> + body { font-family: system-ui; padding: 1rem; } + .error { color: red; } + .preview { margin-top: 1rem; } + pre { background: #f5f5f5; padding: 1rem; border-radius: 4px; } + </style> </head> <body> <a href="${openInAzimuttUrl(aml)}" target="_blank">Open in Azimutt</a> - <pre>${content}</pre> + <div class="preview"> + ${content ? `<pre>${content}</pre>` : '<div class="error">Invalid AML content</div>'} + </div> </body> </html>`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
extensions/vscode-aml/README.md(1 hunks)extensions/vscode-aml/src/web/extension.ts(1 hunks)extensions/vscode-aml/syntaxes/aml.tmLanguage.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/vscode-aml/syntaxes/aml.tmLanguage.json
🧰 Additional context used
🪛 LanguageTool
extensions/vscode-aml/README.md
[uncategorized] ~42-~42: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e made of several and quite independent part. For general knowledge, look at the [ex...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🪛 Markdownlint (0.35.0)
extensions/vscode-aml/README.md
3-3: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🪛 Biome (1.9.4)
extensions/vscode-aml/src/web/extension.ts
[error] 196-196: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (1)
extensions/vscode-aml/src/web/extension.ts (1)
229-266: LGTM! Well-implemented utility functions
The utility functions are well-implemented with proper TypeScript types and error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
extensions/vscode-aml/README.md (3)
3-3: Add alt text to the marketplace badge for accessibility.Add descriptive alt text to the marketplace badge image.
-[](https://marketplace.visualstudio.com/items?itemName=azimutt.vscode-aml) +[](https://marketplace.visualstudio.com/items?itemName=azimutt.vscode-aml)🧰 Tools
🪛 Markdownlint (0.35.0)
3-3: null
Images should have alternate text (alt text)(MD045, no-alt-text)
19-30: Consider structuring the roadmap items.The roadmap items could be better organized by grouping them into categories or phases. Consider:
- Core functionality (diagram preview, conversions)
- Editor features (auto-complete, hover, etc.)
- Advanced features (database connection)
This would help users and contributors understand the development priorities better.
42-42: Fix grammatical error.There's a grammatical error in this sentence.
-VS Code language extensions are made of several and quite independent part. +VS Code language extensions are made of several and quite independent parts.🧰 Tools
🪛 LanguageTool
[uncategorized] ~42-~42: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e made of several and quite independent part. For general knowledge, look at the [ex...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
extensions/vscode-aml/.vscodeignore(1 hunks)extensions/vscode-aml/README.md(1 hunks)extensions/vscode-aml/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- extensions/vscode-aml/.vscodeignore
- extensions/vscode-aml/package.json
🧰 Additional context used
🪛 LanguageTool
extensions/vscode-aml/README.md
[uncategorized] ~42-~42: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e made of several and quite independent part. For general knowledge, look at the [ex...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🪛 Markdownlint (0.35.0)
extensions/vscode-aml/README.md
3-3: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (3)
extensions/vscode-aml/README.md (3)
12-14: LGTM!
The features section is clear and well-documented.
64-64: Reusing existing review comment about access token security.
7-7: Verify the screenshot path.
The screenshot path starts with a forward slash which might not work correctly on GitHub. Consider using a relative path instead.
✅ Verification successful
The screenshot path is valid and accessible
The screenshot exists at the specified path extensions/vscode-aml/assets/screenshot.png. While the path starts with a forward slash, this is actually a common and valid syntax for referencing images in markdown files on GitHub, as GitHub automatically resolves repository-relative paths starting with /. The current path will work correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the screenshot file exists at the specified path
if [ -f "extensions/vscode-aml/assets/screenshot.png" ]; then
echo "Screenshot exists at the correct path"
else
echo "Screenshot not found at the expected path"
fi
Length of output: 141
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (10)
extensions/vscode-aml/README.md (6)
9-9: Use a stable reference for the screenshot URLThe current URL uses
refs/heads/mainwhich might break if the branch structure changes. Consider using a specific commit hash or release tag instead of the branch reference.- +
20-20: Fix grammar in usage instructionsAdd "the" before "documentation".
-Write your schema using AML, check [documentation](https://azimutt.app/docs/aml) is needed. +Write your schema using AML, check the [documentation](https://azimutt.app/docs/aml) if needed.
24-38: Enhance the AML code example with more explanatory commentsConsider adding more inline comments to explain the AML syntax features being demonstrated, such as default values, enums, and timestamps.
```aml users id uuid pk - name varchar index - email varchar unique - role user_role(admin, guest)=guest + name varchar index # demonstrates indexing + email varchar unique # demonstrates unique constraint + role user_role(admin, guest)=guest # demonstrates enum with default value posts | store all posts id uuid pk title varchar content text | allow markdown formatting author uuid -> users(id) # inline relation - created_at timestamp=`now()` + created_at timestamp=`now()` # demonstrates default value with function--- `44-50`: **Separate feature descriptions from implementation details** Consider restructuring the roadmap to separate user-facing feature descriptions from technical implementation details. This makes the roadmap more accessible to users while preserving the technical notes. ```diff -Add parsing errors ([createDiagnosticCollection](https://code.visualstudio.com/api/references/vscode-api#languages.createDiagnosticCollection)?) -auto-complete (cf [registerCompletionItemProvider](https://microsoft.github.io/monaco-editor/typedoc/functions/languages.registerCompletionItemProvider.html)) +### Planned Features +- Syntax error detection and reporting +- Intelligent code completion + +### Technical Implementation Notes +- Use `createDiagnosticCollection` for error reporting +- Implement `registerCompletionItemProvider` for auto-complete
64-64: Fix plural form in development sectionCorrect the noun number for better grammar.
-VS Code language extensions are made of several and quite independent part. +VS Code language extensions are made of several and quite independent parts.🧰 Tools
🪛 LanguageTool
[uncategorized] ~64-~64: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e made of several and quite independent part. For general knowledge, look at the [ex...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
1-90: Improve document structure with consistent heading hierarchyConsider using consistent heading levels throughout the document and adding a table of contents for better navigation.
+# AML Support for VS Code + +## Table of Contents + +- [Features](#features) +- [Usage](#usage) +- [Roadmap](#roadmap) +- [Issues & Contributing](#issues--contributing) +- [Development](#development) + - [Components](#components) + - [Tips](#tips) +- [Publication](#publication) + -# AML Support for VS Code🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: You might be missing the article “the” here.
Context: ...age Create an empty.amlfile or useAML: New database schema (ERD)command....(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~64-~64: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e made of several and quite independent part. For general knowledge, look at the [ex...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
extensions/vscode-aml/src/web/extension.ts (4)
30-30: Consider scoping the previewPanel variableThe
previewPanelvariable is declared in the global scope. Consider moving it inside theactivatefunction and managing its lifecycle there for better encapsulation.-let previewPanel: WebviewPanel | undefined = undefined +export function activate(context: ExtensionContext) { + let previewPanel: WebviewPanel | undefined = undefined
43-43: Add documentation for the deactivate functionEven though the function is empty, it's part of the VSCode extension lifecycle. Add a JSDoc comment explaining why no cleanup is needed.
+/** + * This method is called when the extension is deactivated. + * No cleanup needed as all disposables are managed by VSCode's ExtensionContext. + */ export function deactivate() {}
47-66: Consider making the sample content configurableThe sample content is hardcoded. Consider:
- Loading it from a template file
- Making it configurable through extension settings
- Adding multiple templates for different use cases
Would you like help implementing template configuration support?
257-267: Enhance debounce with cleanup handlingThe current debounce implementation doesn't handle cleanup of pending timeouts.
function debounce<F extends (...args: Parameters<F>) => ReturnType<F>>( func: F, delay: number ): (...args: Parameters<F>) => void { let timeout: Timeout - return (...args: Parameters<F>): void => { + const debouncedFn = (...args: Parameters<F>): void => { clearTimeout(timeout) timeout = setTimeout(() => func(...args), delay) } + // Add cleanup method + debouncedFn.cancel = () => { + clearTimeout(timeout) + } + return debouncedFn }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
extensions/vscode-aml/assets/icon-white.pngis excluded by!**/*.pngextensions/vscode-aml/assets/icon.pngis excluded by!**/*.pngextensions/vscode-aml/assets/screenshot.pngis excluded by!**/*.png
📒 Files selected for processing (3)
extensions/vscode-aml/README.md(1 hunks)extensions/vscode-aml/package.json(1 hunks)extensions/vscode-aml/src/web/extension.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/vscode-aml/package.json
🧰 Additional context used
🪛 LanguageTool
extensions/vscode-aml/README.md
[uncategorized] ~19-~19: You might be missing the article “the” here.
Context: ...age Create an empty .aml file or use AML: New database schema (ERD) command....
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~64-~64: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e made of several and quite independent part. For general knowledge, look at the [ex...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🪛 Biome (1.9.4)
extensions/vscode-aml/src/web/extension.ts
[error] 197-197: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
extensions/vscode-aml/package.json (2)
9-11: Consider lowering the VSCode engine version requirement.The current requirement of
^1.95.0might unnecessarily limit your user base. Many of the features used in this extension might work with older versions of VSCode. Consider testing with lower versions (e.g.,^1.74.0) to support a broader user base.
86-89: Improve clarity of the convert command title.The title "Convert AML to ..." is vague and doesn't indicate the available conversion options. Consider either:
- Listing the supported formats: "Convert AML to (JSON/SQL/...)"
- Or splitting into separate commands for each target format
extensions/vscode-aml/resources/schema.json (1)
90-95: Consider using schema-level comments instead of line numbersLine numbers in comments can become stale when the schema changes. Consider moving these organizational comments to schema-level documentation.
{ "extra": { "comments": [ - {"line": 15, "comment": "CMS tables"}, - {"line": 39, "comment": "Tracking tables"} + {"schema": "cms", "comment": "CMS tables"}, + {"schema": "tracking", "comment": "Tracking tables"} ] } }extensions/vscode-aml/src/web/extension.ts (2)
74-75: Enhance user feedback for work-in-progress featuresWhile the conversion features are being implemented, consider providing more informative feedback:
- Indicate the expected completion timeline
- Show a progress indicator for long-running operations
- Provide alternative workarounds if available
-vscode.window.showInformationMessage('JSON to AML conversion not implemented yet, work in progress...') +await vscode.window.withProgress( + { + location: vscode.ProgressLocation.Notification, + title: "JSON to AML conversion", + cancellable: false + }, + async () => { + await vscode.window.showInformationMessage( + 'JSON to AML conversion is coming soon! In the meantime, you can use the Azimutt web app at https://azimutt.app', + 'Open Azimutt' + ).then(selection => { + if (selection === 'Open Azimutt') { + vscode.env.openExternal(vscode.Uri.parse('https://azimutt.app')); + } + }); + } +);Also applies to: 87-88, 102-124
169-185: Enhance preview functionality with syntax highlightingThe current preview shows plain text content. Consider enhancing it with:
- Syntax highlighting using Prism.js or highlight.js
- Dark theme support based on VSCode's current theme
- Custom CSS for better readability
return `<!DOCTYPE html> <html lang="en"> <head> <meta charset="UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> <title>AML preview</title> + <link href="https://cdnjs.cloudflare.com/ajax/libs/prism/1.24.1/themes/prism.min.css" rel="stylesheet" /> + <style> + body { padding: 16px; } + pre { margin: 0; } + .vscode-dark pre { background: #1e1e1e; } + </style> </head> <body class="${panel.webview.options.localResourceRoots ? 'vscode-dark' : 'vscode-light'}"> <a href="${openInAzimuttUrl(aml)}" target="_blank">Open in Azimutt</a> - <pre>${content}</pre> + <pre><code class="language-aml">${content}</code></pre> + <script src="https://cdnjs.cloudflare.com/ajax/libs/prism/1.24.1/prism.min.js"></script> </body> </html>`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
backend/lib/azimutt_web/templates/website/docs/create-your-project.html.heex(4 hunks)extensions/vscode-aml/package.json(1 hunks)extensions/vscode-aml/resources/schema.json(1 hunks)extensions/vscode-aml/resources/schema.sql(1 hunks)extensions/vscode-aml/src/web/extension.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/lib/azimutt_web/templates/website/docs/create-your-project.html.heex
- extensions/vscode-aml/resources/schema.sql
🧰 Additional context used
🪛 Biome (1.9.4)
extensions/vscode-aml/src/web/extension.ts
[error] 197-197: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (8)
extensions/vscode-aml/package.json (1)
6-6: Verify presence of referenced files.
The package.json references several files that are crucial for the extension. Let's verify they exist:
Also applies to: 50-51, 53-53, 60-60, 66-66
✅ Verification successful
All referenced files are present and correctly located
All files referenced in package.json exist in their expected locations within the extensions/vscode-aml directory:
- ✓ assets/icon.png
- ✓ assets/icon.svg
- ✓ language-configuration.json
- ✓ syntaxes/aml.tmLanguage.json
- ✓ snippets.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all referenced files exist
echo "Checking referenced files..."
files=(
"assets/icon.png"
"assets/icon.svg"
"language-configuration.json"
"syntaxes/aml.tmLanguage.json"
"snippets.json"
)
for file in "${files[@]}"; do
if [ -f "$file" ]; then
echo "✓ Found: $file"
else
echo "✗ Missing: $file"
fi
done
Length of output: 858
Script:
#!/bin/bash
# Let's check if these files exist in different locations within the extensions/vscode-aml directory
cd extensions/vscode-aml 2>/dev/null
echo "Checking in extensions/vscode-aml directory..."
files=(
"assets/icon.png"
"assets/icon.svg"
"language-configuration.json"
"syntaxes/aml.tmLanguage.json"
"snippets.json"
)
for file in "${files[@]}"; do
if [ -f "$file" ]; then
echo "✓ Found: $file"
else
echo "✗ Missing: $file"
fi
done
# Let's also use fd to find these files anywhere in the repository
echo -e "\nSearching for files in the repository..."
fd -t f "icon\.(png|svg)$|language-configuration\.json$|aml\.tmLanguage\.json$|snippets\.json$"
Length of output: 1199
extensions/vscode-aml/resources/schema.json (4)
26-44: Past review comment is still applicable
The previous review comment about adding indexes for author and status fields remains valid.
46-58: Past review comment is still applicable
The previous review comment about adding indexes for post_id and created_at remains valid.
59-71: Past review comment is still applicable
The previous review comment about adding indexes and constraints for the events entity remains valid.
84-89: LGTM! Types are well-defined
The custom types are properly scoped and have appropriate values defined.
extensions/vscode-aml/src/web/extension.ts (3)
32-41: LGTM! Well-structured command registration
The command registration follows VSCode extension best practices:
- Proper use of context subscriptions for cleanup
- Clear command naming convention
- Appropriate command handlers
257-267: LGTM! Well-implemented debounce utility
The debounce implementation is:
- Type-safe with proper generic constraints
- Correctly handles cleanup of previous timeouts
- Uses appropriate TypeScript types
16-28:
Address module compatibility issues with @azimutt packages
The commented imports are required for implementing the core functionality, but they're causing TypeScript compiler errors due to CommonJS and ECMAScript module incompatibilities. Consider:
- Update the imports to use the
.jsextension explicitly:
-// import {ParserError, ParserErrorLevel} from "@azimutt/models";
-/*import {generateSql, parseSql} from "@azimutt/parser-sql";
+import {ParserError, ParserErrorLevel} from "@azimutt/models/dist/types.js";
+import {generateSql, parseSql} from "@azimutt/parser-sql/dist/index.js";- Update
tsconfig.jsonto use"moduleResolution": "node16"or"nodenext".
Blocked (need help)
When running
tsc --noEmit, I get many error that likely comes from CommonJS / ESM incompatibilities but maybe also something else.They are generated when I import anything from
@azimutt/aml(cf npm) into vscode-aml.Not sure what to change in
vscode-amlor in@azimutt/aml(cf ./libs/aml), main files:Any idea?
Summary by CodeRabbit
Release Notes
New Features
Documentation Updates
Configuration Changes
.gitignoreand.dockerignorefiles to refine ignored files and directories.package.jsonandpnpm-workspace.yamlto reflect the new directory structure for the desktop and browser extension.Bug Fixes