vscode-objectscript
vscode-objectscript copied to clipboard
Client-side editing overhaul
- Fixes #1278.
- Create an index of all Classes, MAC and INT routines, and Include files inside non-isfs workspace folders:
- Create two maps: one with the document name as the key and an array of
Uris as the value, and one in the reverse direction. - Use a
FileSystemWatcherto update the index when files are created, deleted or changed on disk and anonDidChangeTextDocumenthandler to update the index when a file is changed within VS Code. - Delete a file on the server if the last
Urifor it in a workspace folder was deleted. DocumentContentProvider.getUri()will use the index to determine theUrifor a class or routine in a non-isfs folder. The export settings are now only used for non-classes and routines in folders with thefilescheme.WorkspaceSymbolProviderwill only return results from classes that are in the client-side folder by using the index to determine all known classes.
- Create two maps: one with the document name as the key and an array of
- Automatically show and hide the InterSystems Explorer and Projects Explorer views based on the folders in the workspace. InterSystems Explorer is only shown if there is at least one non-isfs(-readonly) workspace folder. Projects Explorer is only shown if there is at least one isfs(-readonly). Both are shown if no workspace folders are open. This formally ties the InterSystems Explorer to client-side editing workflows and Projects Explorer to server-side editing workflows. This replaces the
objectscript.showExplorersetting. - Cache the list of Studio Abstract Document extensions that are supported for each server connection so we can properly import them from client-side folders. Importing abstract documents is now independent of the
objectscript.exportsettings, except for DFI’s which still check the export settings to preserve the path splitting behavior added by #808. Any file within a workspace folder that has a supported abstract document extension will be imported with the last part of the path used as the server name (except for DFI’s that match the export settings). - Don’t allow creation of an editable server-side folder via the
Add Server Namespace to Workspace…command if the current workspace already has a client-side folder in it. This is to prevent the blending of client-side and server-side workflows in a way that can lead to lost changes or overwritten edits. - Change the default value of the
objectscript.explorer.alwaysShowServerCopysetting to true. The InterSystems Explorer should always show the server copy since the local copy can be opened from the files explorer. - Remove the
objectscript.export.noStoragesetting. Storage is an integral part of the class definition, and we shouldn’t allow removing it. - Remove the
objectscript.export.dontExportIfNoChangessetting. I don’t think that turning it on adds any benefit to the user. - Check if the file system is writable before exporting or creating a new file.
- Don’t show server-side source control menu options in the InterSystems Explorer.
- Show MultiValue routines in the InterSystems Explorer.
- Hide .bpl and .dtl documents in the InterSystems Explorer since they can’t be opened or edited.
- Create a single error handling method for consistent error reporting.
- Remove unneeded type definitions.
- Upgrade ancient dev dependencies.
@gjsjohnmurray @isc-rsingh I've opened this as a draft since this is a giant PR that needs thorough review. I have a few design questions for you that I'd like to resolve before formally starting the review process and seeking feedback from other users:
- Should I just remove
objectscript.explorer.alwaysShowServerCopy? Keeping the setting gives users who liked the previous behavior a way to retain it, but is that behavior desirable? I can’t think of a good reason why you’d want the InterSystems Explorer to show the local copies other than “that’s the way it is by default”.
I've decided to keep this setting for backwards compatibility.
- Should I remove the special DFI folder-splitting on export added by #808? Doing so would be backwards incompatible, and that behavior is almost three years old. However, I doubt anyone is using it and it would make sense to treat all Studio Abstract Documents equally.
I've decided to keep this behavior for backwards compatibility.
- Should the automatic deletion of local files on the server be guarded by a setting so users can disable it? Maybe
objectscript.deleteOnFileDelete, modeled afterobjectscript.importOnSave?
This is covered by objectscript.syncLocalChanges.
@daimor I'd appreciate your feedback on this as a regular client-side editing user.
Is this worth publicizing on DC for feedback before merging?
@isc-rsingh I've already done so: https://community.intersystems.com/post/do-you-use-client-side-editing-vs-code-if-so-we-want-your-feedback
FYI after I get a little more user feedback I will be marking this as ready for review. I made the follow architecture changes in my last commit:
- Removed our
onDidSaveTextDocumenthandler in favor of using the new client-side file system watcher for saving changes. I added code inFileSystemProviderto actually save files. - Folded all explorer nodes into a single file to fix module resolution issues I was getting due to circular dependencies between the separate files.
- I improved error handling inside
checkConnection().
@isc-rsingh @gjsjohnmurray I've marked this as ready foe review since I suspect that it will take some time to review. My plan after this is merged is to release a version 3.0.0 including this and implementations for at least the other issues currently assigned to me. I would like to have that out in early 2025.