Field multiplicity #39
Conversation
Gaviola
requested review from
angel-perez-itti,
Copilot,
dgaviola,
jguardiolagarcia,
lucianomasuelli and
sebasmoyano
There was a problem hiding this comment.
Pull Request Overview
This pull request implements field multiplicity conversion capabilities, allowing developers to transform model fields between single value and array types through VS Code commands. The feature includes automatic name pluralization/singularization, type annotation updates, and comprehensive reference updating throughout the codebase.
- Added refactor tools for converting fields to/from array types with intelligent naming conventions
- Enhanced UI context handling to display appropriate commands based on field type (single vs array)
- Updated metadata handling to preserve array notation in type names
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/metadata.ts | Adds isFieldMultiple utility function to detect array field types |
| src/refactor/tools/changeFieldToSingleValue.ts | Implements tool for converting array fields to single values with name singularization |
| src/refactor/tools/changeFieldToArray.ts | Implements tool for converting single fields to arrays with name pluralization |
| src/refactor/refactorInterfaces.ts | Adds payload type definitions for field multiplicity conversion operations |
| src/refactor/refactorDisposables.ts | Registers the new refactor tools in the system |
| src/explorer/appTreeItem.ts | Updates context value assignment to differentiate between single and array fields |
| src/cache/cache.ts | Modifies getCleanTypeName to preserve array notation in type names |
| package.json | Adds new commands and updates context menu conditions for field type-specific operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| // Singularize name if it's plural and update all references | ||
| if (field.name.endsWith('s')) { |
There was a problem hiding this comment.
The singularization logic is overly simplistic. Simply removing the 's' suffix will not work correctly for irregular plurals (e.g., 'children' -> 'childre', 'people' -> 'peopl'). Consider using a proper pluralization library or implementing more sophisticated rules.
| } | ||
|
|
||
| // Pluralize name if it's not already plural and update all references | ||
| if (!field.name.endsWith('s')) { |
There was a problem hiding this comment.
The pluralization logic is overly simplistic. Simply adding an 's' suffix will not work correctly for words that require different pluralization rules (e.g., 'child' -> 'childs' instead of 'children', 'person' -> 'persons' instead of 'people'). Consider using a proper pluralization library or implementing more sophisticated rules.
| // Change the field's type to an array type | ||
| const document = await vscode.workspace.openTextDocument(declaration.uri); | ||
| const lineText = document.lineAt(declaration.range.start.line).text; | ||
| const typeRegex = new RegExp(`:\\s*${field.type}`); |
There was a problem hiding this comment.
The field.type is used directly in a regex without escaping special characters. If the type contains regex special characters (e.g., 'Map<string, number>'), this will create an invalid regex or match unintended text. Use escapeRegExp like in the changeFieldToSingleValue tool.
dgaviola
left a comment
dgaviola
left a comment
There was a problem hiding this comment.
I have to agree with el bicho. I think we can use a library to make it easy to find the plural of a word and the other way around. Apart from that, it looks good. The only other thing is the refactoring with AI, which asks for too many confirmations. This is something I had noticed in other cases as well. But I guess this is something we need to review in another issue about how to improve it.
[FEAT] [ISSUE #30] Array Persistence for SQL Databases
This pull request adds support for changing a model field between a single value and an array type. It introduces new context values for fields, updates the UI commands and context menus, and implements the refactor's logic to perform these changes, including updating field names and references throughout the codebase.
Field Type Conversion Features:
slingr-vscode-extension.changeFieldToArrayandslingr-vscode-extension.changeFieldToSingleValueto the extension, enabling conversion of fields between single value and array types via the UI.ChangeFieldToArrayToolandChangeFieldToSingleValueToolrefactor tools, including logic to update type annotations, pluralize field names, and update all references when converting a field to an array or back to a single value.Context and UI Improvements:
AppTreeItemto assign context valuesfieldSingleandfieldArraybased on the field's type, enabling context-sensitive commands in the explorer UI and ensuring correct icons are displayed.package.jsonso that field-related commands (rename, change type, delete, convert to array/single) are only available for the appropriate field type.Type Handling and Refactor Infrastructure:
getCleanTypeNameinMetadataCacheto preserve array notation when displaying type names, ensuring accurate type representations.ChangeFieldToArrayPayload,ChangeFieldToSingleValuePayload) in the refactor infrastructure.