Skip to content

fix typescript organizeImports settings #232676

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.

Sign up for GitHub

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

Jump to bottom
Merged
mjbvz merged 3 commits into from
Oct 30, 2024
Merged

fix typescript organizeImports settings #232676

mjbvz merged 3 commits into from
Oct 30, 2024

Conversation

@iisaduan
Copy link
Member

@iisaduan iisaduan commented Oct 30, 2024 •
edited
Loading

Fixes #59942
Fixes #225814 ?
Similar to the fix #227011

When added, some settings had no descriptions in extensions/typescript-language-features/package.json so they weren't resolving. The settings that did have descriptions didn't work because one of the broken settings needs to be specified, otherwise the remaining settings are ignored in the TS LS.

I added those descriptions, and also edited some of the existing ones for better documentation on what the settings are intended to specify. I also refactored the function getOrganizeImportsPreferences to be more evident that many settings are ignored unless unicodeCollation: unicode is specified, and to align the default values more closely to typescript's UserPreferences object.

@iisaduan iisaduan changed the title add descriptions, refactor getPreferences function to be more clear fix typescript organizeImports settings Oct 30, 2024
@iisaduan
Copy link
Member Author

iisaduan commented Oct 30, 2024 •
edited
Loading

cc @mattbierner, I verified in a local build that the settings are correctly being passed into tsserver, and their effects do show up when calling Organize Imports. Admittedly, in many situations, some of the settings have pretty subtle effects, or their name may be misleading (example), so if you have any more ideas/suggestions for where to put descriptions, please let me know. Currently, I think the only place to see the description is hovering over the definition in settings.json, and it only shows the description for the option currently specified.

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 30, 2024

Thanks for taking a look! I do see it working now so will merge the pr but I agree way the settings work is pretty confusing, especially how many settings only when other ones enabled. Would be great to get a few presets enabled too as I think that's what users will end up using in almost all cases: microsoft/TypeScript#59579

@mjbvz mjbvz enabled auto-merge October 30, 2024 23:30
@vs-code-engineering vs-code-engineering bot added this to the November 2024 milestone Oct 30, 2024
@mjbvz mjbvz merged commit ea6e8e2 into microsoft:main Oct 30, 2024
@pmachapman pmachapman mentioned this pull request Dec 12, 2024
Merged
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Dec 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mjbvz mjbvz mjbvz approved these changes

@justschen justschen justschen approved these changes

Assignees

@egamma egamma

Labels

None yet

Projects

None yet

Milestone

November 2024

Development

Successfully merging this pull request may close these issues.

Organize imports preferences don't seem to work Add settings for typescript's organizeImports

4 participants

@iisaduan @mjbvz @justschen @egamma