Skip to content

Conversation

@Cosifne
Copy link
Member

@Cosifne Cosifne commented Apr 9, 2024

This is a test-only change.
Originally mentioned by Sam somewhere we need some tests to guard the registration files of Unified Settings.
This PR adds unit test to cover the C# Intellisense page.
It covers

  1. Type
  2. Default value
  3. The storage path to ISettingManager. Make sure the value is correct and matches
    public static readonly IReadOnlyDictionary<string, VisualStudioOptionStorage> Storages = new Dictionary<string, VisualStudioOptionStorage>()
  4. The enum to int mapping if the option type is Enum

This would ensure we don't make simple mistakes like typo in the registration file.

The general test code is put under Microsoft.VisualStudio.LanguageServices.UnitTests, and the real test is put under Microsoft.VisualStudio.LanguageServices.CSharp.UnitTests, so that later we can onboard the VB tests easily.

@Cosifne Cosifne requested a review from a team as a code owner April 9, 2024 00:14
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 9, 2024
@Cosifne
Copy link
Member Author

Cosifne commented Apr 9, 2024

Kindly tag @sharwell to review.


' Mapping from the config name to its path in Unified Settings registration file
Private Shared ReadOnly s_unifiedSettingsStorage As New Dictionary(Of String, UnifiedSettingsStorage)() From {
{"dotnet_trigger_completion_on_typing_letters", New UnifiedSettingsStorage("textEditor.%LANGUAGE%.intellisense.triggerCompletionOnTypingLetters")},
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 Is this mapping not anywhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, not now at least.

Copy link
Member Author

Choose a reason for hiding this comment

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

They might be moved to

public static readonly IReadOnlyDictionary<string, VisualStudioOptionStorage> Storages = new Dictionary<string, VisualStudioOptionStorage>()
in future
Unified Settings provides Microsoft.VisualStudio.Utilities.UnifiedSettings.ISettingsManager, which would replace the usage of all old ISettingsManager2 IVsFeatureFlag and etc... but it's not required now.

@Cosifne Cosifne requested a review from sharwell April 9, 2024 19:52
@Cosifne Cosifne enabled auto-merge April 11, 2024 18:48
@Cosifne
Copy link
Member Author

Cosifne commented Apr 12, 2024

/azp run roslyn-integratoin-CI

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants