Skip to content

Conversation

@chris-a-talbot
Copy link
Collaborator

PR 3/? for expanded, refactored SLiM vscode extension, addressing issue #12

This PR shouldn't change much (potentially any) functionality; it only breaks up the language server index.ts into a much more modular format, which will be expanded upon in future versions.

Refactor: Increase modularity of language server providers and utils for future expansion
    server/src for all language server code
    server/src/config for constants and types shared across the language server
        config.ts for constants
        paths.ts for paths
        types.ts for TypeScript types
    server/src/handlers holds handlers.ts for implementing handlers that get sent to server/index.ts
    server/src/providers for providers of individual language server features (fed into handlers)
        completion.ts for code completion
        document-symbols.ts for outline view
        hover.ts for hover info
        references.ts for finding all references across the file (not yet implemented)
        signature-help.ts for hints about required & optional parameters for functions and methods
    server/src/services for services used across the language server
        documentation-service.ts manages loading documentation
        validation-service.ts manages validating code files / checking for errors
    server/src/utils for general utilities used across the language server
        instance.ts for tracking defined objects within the current file
        positions.ts for tracking positions in the file
    server/src/validation for scripts used by the validation service to check for errors
        structure.ts for validating the structure of the script (e.g. semicolon-related errors)

Copy link
Collaborator

@andrewkern andrewkern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a great refactor. thanks for splitting this all up! I've put in a couple minor comments / thoughts for you. What do you think? Neither is a blocker and we could just merge this and keep going

import * as fs from 'fs';
import { ClassInfo } from '../config/types';
import { EIDOS_CLASSES_PATH, SLIM_CLASSES_PATH } from '../config/paths';

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the fault of my original implementation-- we are doing duplicate file reads for class constructors. i wonder if could use the already-loaded classesData from the documentation service instead? what do you think? not a blocker for this PR though

// Export the instance to class map from config
export const instanceToClassMap = INSTANCE_TO_CLASS_MAP;

export let instanceDefinitions: { [key: string]: string } = {};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again an issue with my original implementation-- I don't think the instanceDefinitions are being reset between documents. potentially this could lead to stale data when a user switches files. maybe we should reset instanceDefinitions at the start of trackInstanceDefinitions() or per-document?

@chris-a-talbot
Copy link
Collaborator Author

@andrewkern Future updates expand on the completion service and instance tracking substantially, and I believe those updates resolve both of these problems - if you're alright with it, I think it'd be easiest to merge this version as-is if there are no other issues, and then resolve these issues in the PRs that expand upon each of these features?

@andrewkern
Copy link
Collaborator

okay sounds good to me. I'll get this merged

@andrewkern andrewkern merged commit b163204 into slim-community:main Nov 27, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants