forked from kevb34ns/auto-comment-blocks
-
Notifications
You must be signed in to change notification settings - Fork 1
Fix typings and add interfaces #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
yCodeTech
wants to merge
15
commits into
master
Choose a base branch
from
fix/typings
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added new `Settings` interface to define all the configuration settings and their typings. - Refactored the `Configuration::getConfigurationValue` method to use the new `Settings` interface and automatically get the correct typing of the specified key from the interface. This also enables intellisense support for the settings keys. It also removes the requirement of hardcoding the type via generic typing when using the method. ie. `getConfigurationValue<string[]>()`. - Removed all generic typings from the `getConfigurationValue` method calls.
- Fixed the typing of `overrideDefaultLanguageMultiLineComments` setting to use the `Record` utility type. The setting is defined in the package.json as an object, but it has always been wrongly typed as an array (`string[]`). The utility type `Record` defines the types for the keys and values, assuming all keys and all values will be of the same typing, ie stringed keys and numeric values, in our case though it's stringed values. It's an easier way of typing it as `{ [ key: string ]: string }`.
- Removed the import for the `SupportUnsupportedLanguages` interface, as this isn't available until 1.2.0. This was wrongly added to this PR while adding it for the 1.2.0 version.
- Removed the generic typing from the `Configuration::getConfigurationValue` method call in extension.ts file.
- Changed the job name in check-ts.yml from `deploy` to `check-ts`, as it's confusing seeing `deploy` on GitHub PR's and actions when it actually isn't the deploy workflow. This workflow only checks ts errors.
- Merge `ExtensionData::createExtensionData` method into `ExtensionData::setExtensionData` method for simplicity. We once had a unified method before until I split them into the 2 aforementioned methods in PR #12 (commit b0a40aa). Not sure why they were split, maybe because I thought too many `this.extensionData.set` was duplication, but it actually isn't. So we now merge them back.
- Added new `ExtensionMetaData` interface for better typings and vscode intellisense. - Changed the `extensionData` property map to use typing of the new `ExtensionMetaData` interface. - Changed `get` and `getAll` methods to use the typing of `ExtensionMetaData` interface. This is instead of using return type of the old `createExtensionData` method, and still allows vscode intellisense of the data properties. It also is a better way to type the properties.
- Refactor `Configuration::setBladeComments` method to remove the duplicated blade and html comment arrays. We now only define the arrays once as a variable and use it in the return statement. - Fix `setBladeComments` method return type to vscode's `CharacterPair` type. Also typed the blade and html comments variables as `CharacterPair`.
- Fixed the type of the `value` parameter of `Configuration::updateConfigurationValue` method to use `unknown` instead of `any`. This is because `any` type just disables TypeScript's error reporting, while `unknown` is a type-safe usage for "anything". Using `unknown` type is generally preferred over `any`.
- Fixed the type for the `config` param to vscode's `LanguageConfiguration` instead of `any` in both `setMultiLineCommentLanguageDefinitions` and `setSingleLineCommentLanguageDefinitions` methods. - Typed the `langArray` variable in `setMultiLineCommentLanguageDefinitions` method as `string[]`. - Fixed `defaultMultiLineConfig` variable's type to vscode's `LanguageConfiguration` instead of `any` in `setLanguageConfiguration`.
Previously, the `lineComment` style check in `setSingleLineCommentLanguageDefinitions` used the string `includes` method to determine if the style "included" a `;` because they could also be `;;`. But since those are the only 2 strings that it can be, it makes more sense to just do a strict equality check for both instead. - Refactored `lineComment` style conditional to remove the `includes` method and use strict equality checks for `;` OR `;;`.
…ons`
- Added a new custom type, `SingleLineCommentStyle`, in the new `commentStyles` file to define 3 union strings for the single line styles.
- Fix `style` variable type in `setSingleLineCommentLanguageDefinitions` to the union type of the new `SingleLineCommentStyle` type and `null`. This gives more info to ts for what `style` should be, either 1 of the 3 strings or `null`. By typing the 3 style options ("//", "#", ";"), we are also enabling vscode's intellisense to auto complete the value. This helps for future development.
- Changed the default value for `style` variable to be `null` instead of an empty string. This is so that we don't need to type the new `SingleLineCommentStyle` as `""` for empty string which looks weird as a union type. It also provides a better understanding of why the variable might be null, ie. an unsupported style.
Also changed the style empty string checks to null checks.
- Fixed typing for the language config `lineComment` property.
The property used to be solely a string, but as of June 2025, VScode changed it to be a string, object or null, in the PR microsoft/vscode#243283, specifically commit microsoft/vscode@d9145a2. However, this has not been updated in the npm package `@types/vscode` by DefinitelyTyped, so ts still thinks it should be a string only. So when we try to access the `comment` property in the object, then ts spits out errors.
While I created an issue for it (DefinitelyTyped/DefinitelyTyped#74372), we need need to manually add the changes until @types/vscode is changed accordingly.
- Added new `LineComment` custom type in the new commentStyles file to be a union of `string`, `LineCommentConfig`, and `null`. This typing is inline with vscode's change, but as a custom type instead of inside an interface. Also typed the `lineComment` variable as this new type and cast the `config.comments.lineComment` as this type too, to avoid any ts errors.
- Added new `LineCommentConfig` interface in the new commentStyles file, to provide info for the object options. This is inline with vscode's change.
- Changed the conditional for the `lineComment` as an object to first check it is of type object and not null before checking if `comment` property exists in the object. This prevents ts erroring.
- Fixed ts type error `Type 'void | CharacterPair' is not assignable to type 'CharacterPair'` while trying to set blade comments in `setLanguageConfiguration` method. This probably happens because the `setBladeComments` method is returning void and being set directly to the lang config `blockComment` property which doesn't work. So to avoid this, we set the blade comments into a new variable instead and then only set the `blockComment` property if the conditional check of `bladeComments` variable is truthy, ie. has any value except null,undefined, etc.
- Removed the unused duplicate `style` variable in `setSingleLineCommentLanguageDefinitions` method.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request refactors how configuration values are accessed in the
Configurationclass to improve type safety and maintainability. The main change is the introduction of a newSettingsinterface that defines all supported configuration keys and their types, and the update of thegetConfigurationValuemethod to use this interface for type-safe access throughout the codebase.Type Safety and Configuration Refactor:
Settingsinterface insrc/interfaces/settings.tsthat explicitly defines all available configuration keys and their respective types.getConfigurationValuemethod insrc/configuration.tsto use generic key access (K extends keyof Settings) and return the correct type fromSettings, ensuring type-safe retrieval of configuration values.getConfigurationValueinsrc/configuration.tsto remove explicit type arguments and leverage the new type-safe method signature.