Skip to content

Remove rip-cord feature flags #77738

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

Merged
merged 5 commits into from
Mar 21, 2025
Merged

Remove rip-cord feature flags #77738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a64c0d
Remove rip-cord feature flags
CyrusNajmabadi Mar 21, 2025
876a47a
Merge remote-tracking branch 'upstream/release/dev17.15' into removeF…
CyrusNajmabadi Mar 21, 2025
3fffc83
Merge remote-tracking branch 'upstream/release/dev17.15' into removeF…
CyrusNajmabadi Mar 21, 2025
80e391a
Merge remote-tracking branch 'upstream/release/dev17.15' into removeF…
CyrusNajmabadi Mar 21, 2025
76f6859
Fix tests
CyrusNajmabadi Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/EditorFeatures/Core/InlineRename/InlineRenameSessionOptionsStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ internal static class InlineRenameSessionOptionsStorage
public static readonly Option2<bool> RenameFile = new("dotnet_rename_file", defaultValue: true);
public static readonly Option2<bool> PreviewChanges = new("dotnet_preview_inline_rename_changes", defaultValue: false);

public static readonly Option2<bool?> CommitRenameAsynchronously = new("dotnet_commit_rename_asynchronously", defaultValue: null);
public static readonly Option2<bool> CommitRenameAsynchronouslyFeatureFlag = new("dotnet_commit_rename_asynchronously_feature_flag", defaultValue: false);
public static readonly Option2<bool> CommitRenameAsynchronously = new("dotnet_commit_rename_asynchronously", defaultValue: true);

public static bool ShouldCommitAsynchronously(this IGlobalOptionService globalOptionService)
=> globalOptionService.GetOption(CommitRenameAsynchronously) ?? globalOptionService.GetOption(CommitRenameAsynchronouslyFeatureFlag);
=> globalOptionService.GetOption(CommitRenameAsynchronously);
}
20 changes: 18 additions & 2 deletions src/EditorFeatures/Test2/Rename/RenameCommandHandlerTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Imports Microsoft.CodeAnalysis.Editor.Implementation.InlineRename
Imports Microsoft.CodeAnalysis.Editor.InlineRename
Imports Microsoft.CodeAnalysis.Editor.Shared.Extensions
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions
Imports Microsoft.CodeAnalysis.InlineRename
Imports Microsoft.CodeAnalysis.Options
Imports Microsoft.CodeAnalysis.Text.Shared.Extensions
Imports Microsoft.VisualStudio.Commanding
Expand Down Expand Up @@ -1043,6 +1044,9 @@ partial class [|Program|]
</Project>
</Workspace>, host)

Dim globalOptions = workspace.GetService(Of IGlobalOptionService)
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronously, False)

Dim view = workspace.Documents.Single().GetTextView()

Dim commandHandler = CreateCommandHandler(workspace)
Expand Down Expand Up @@ -1167,7 +1171,10 @@ partial class [|Program|]
End Sub)
End Sub

Private Shared Sub VerifyCommandCommitsRenameSessionAndExecutesCommand(host As RenameTestHost, executeCommand As Action(Of RenameCommandHandler, IWpfTextView, Action))
Private Shared Sub VerifyCommandCommitsRenameSessionAndExecutesCommand(
host As RenameTestHost,
executeCommand As Action(Of RenameCommandHandler, IWpfTextView, Action),
Optional commitAsynchronously As Boolean = False)
Using workspace = CreateWorkspaceWithWaiter(
<Workspace>
<Project Language="C#" CommonReferences="true">
Expand All @@ -1181,6 +1188,9 @@ class [|C$$|]
</Project>
</Workspace>, host)

Dim globalOptions = workspace.GetService(Of IGlobalOptionService)
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronously, commitAsynchronously)

Dim view = workspace.Documents.Single().GetTextView()
view.Caret.MoveTo(New SnapshotPoint(view.TextBuffer.CurrentSnapshot, workspace.Documents.Single(Function(d) d.CursorPosition.HasValue).CursorPosition.Value))

Expand Down Expand Up @@ -1241,7 +1251,10 @@ class [|C$$|]
End Using
End Function

Private Shared Sub VerifySessionCommittedAfterCutPasteOutsideIdentifier(host As RenameTestHost, executeCommand As Action(Of RenameCommandHandler, IWpfTextView, Action))
Private Shared Sub VerifySessionCommittedAfterCutPasteOutsideIdentifier(
host As RenameTestHost,
executeCommand As Action(Of RenameCommandHandler, IWpfTextView, Action),
Optional commitAsynchronously As Boolean = False)
Using workspace = CreateWorkspaceWithWaiter(
<Workspace>
<Project Language="C#" CommonReferences="true">
Expand All @@ -1255,6 +1268,9 @@ class [|C$$|]
</Project>
</Workspace>, host)

Dim globalOptions = workspace.GetService(Of IGlobalOptionService)
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronously, commitAsynchronously)

Dim view = workspace.Documents.Single().GetTextView()
view.Caret.MoveTo(New SnapshotPoint(view.TextBuffer.CurrentSnapshot, workspace.Documents.Single(Function(d) d.CursorPosition.HasValue).CursorPosition.Value))

Expand Down
16 changes: 4 additions & 12 deletions src/LanguageServer/Protocol/Features/Options/SolutionCrawlerOptionsService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,10 @@
namespace Microsoft.CodeAnalysis.SolutionCrawler;

[ExportWorkspaceService(typeof(ISolutionCrawlerOptionsService)), Shared]
internal sealed class SolutionCrawlerOptionsService : ISolutionCrawlerOptionsService
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class SolutionCrawlerOptionsService(IGlobalOptionService globalOptions) : ISolutionCrawlerOptionsService
{
private readonly IGlobalOptionService _globalOptions;

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public SolutionCrawlerOptionsService(IGlobalOptionService globalOptions)
{
_globalOptions = globalOptions;
}

public bool EnableDiagnosticsInSourceGeneratedFiles
=> _globalOptions.GetOption(SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFiles) ??
_globalOptions.GetOption(SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFilesFeatureFlag);
=> globalOptions.GetOption(SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFiles);
Copy link
Member

Choose a reason for hiding this comment

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

So I think we might need a bigger approach for ripcord settings like this, since I think this PR as written puts is in a worse place. temporarily. Right now it's obvious this was a feature flag rollout, but once we delete it then this setting lives around even longer...and it's not obvious anymore (unless you have the context for it) that the only reason the setting exists was for a ripcord -- no user should need to change it otherwise.

I'd say we should be deleting these switches (and the commit async and document outline flags) entirely, or add some sort of TODO on them, or check in a docs file listing the flags and the expected removal date. Because otherwise this will still live around until somebody notices them "eventually".

Copy link
Member Author

Choose a reason for hiding this comment

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

i'm ok pulling the flags entirely.

}
7 changes: 2 additions & 5 deletions src/LanguageServer/Protocol/Features/Options/SolutionCrawlerOptionsStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@ internal static class SolutionCrawlerOptionsStorage
public static readonly PerLanguageOption2<CompilerDiagnosticsScope> CompilerDiagnosticsScopeOption = new(
"dotnet_compiler_diagnostics_scope", defaultValue: CompilerDiagnosticsScope.OpenFiles, group: s_backgroundAnalysisOptionGroup, serializer: EditorConfigValueSerializer.CreateSerializerForEnum<CompilerDiagnosticsScope>());

public static readonly Option2<bool?> EnableDiagnosticsInSourceGeneratedFiles = new(
"dotnet_enable_diagnostics_in_source_generated_files", defaultValue: null, group: s_backgroundAnalysisOptionGroup);

public static readonly Option2<bool> EnableDiagnosticsInSourceGeneratedFilesFeatureFlag = new(
"dotnet_enable_diagnostics_in_source_generated_files_feature_flag", defaultValue: false, group: s_backgroundAnalysisOptionGroup);
public static readonly Option2<bool> EnableDiagnosticsInSourceGeneratedFiles = new(
"dotnet_enable_diagnostics_in_source_generated_files", defaultValue: true, group: s_backgroundAnalysisOptionGroup);

/// <summary>
/// Enables forced <see cref="BackgroundAnalysisScope.Minimal"/> scope when low VM is detected to improve performance.
Expand Down
18 changes: 3 additions & 15 deletions src/VisualStudio/CSharp/Impl/Options/AdvancedOptionPageControl.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,22 +68,15 @@ public AdvancedOptionPageControl(OptionStore optionStore) : base(optionStore)
BindToOption(Automatic_Run_generators_after_any_change, WorkspaceConfigurationOptionsStorage.SourceGeneratorExecution, SourceGeneratorExecutionPreference.Automatic);
BindToOption(Balanced_Run_generators_after_saving_or_building, WorkspaceConfigurationOptionsStorage.SourceGeneratorExecution, SourceGeneratorExecutionPreference.Balanced);

BindToOption(Analyze_source_generated_files, SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFiles, () =>
{
// If the option has not been set by the user, check if the option is enabled from experimentation. If so, default to that.
return optionStore.GetOption(SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFilesFeatureFlag);
});
BindToOption(Analyze_source_generated_files, SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFiles);

// Go To Definition
BindToOption(Enable_navigation_to_sourcelink_and_embedded_sources, MetadataAsSourceOptionsStorage.NavigateToSourceLinkAndEmbeddedSources);
BindToOption(Enable_navigation_to_decompiled_sources, MetadataAsSourceOptionsStorage.NavigateToDecompiledSources);
BindToOption(Always_use_default_symbol_servers_for_navigation, MetadataAsSourceOptionsStorage.AlwaysUseDefaultSymbolServers);

// Rename
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.CommitRenameAsynchronously, () =>
{
return optionStore.GetOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronouslyFeatureFlag);
});
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.CommitRenameAsynchronously);
BindToOption(Rename_UI_setting, InlineRenameUIOptionsStorage.UseInlineAdornment, label: Rename_UI_setting_label);

// Using Directives
Expand Down Expand Up @@ -183,12 +176,7 @@ public AdvancedOptionPageControl(OptionStore optionStore) : base(optionStore)
BindToOption(AutomaticallyOpenStackTraceExplorer, StackTraceExplorerOptionsStorage.OpenOnFocus);

// Document Outline
BindToOption(EnableDocumentOutline, DocumentOutlineOptionsStorage.EnableDocumentOutline, () =>
{
// If the option has not been set by the user, check if the option is disabled from experimentation. If
// so, default to reflect that.
return !optionStore.GetOption(DocumentOutlineOptionsStorage.DisableDocumentOutlineFeatureFlag);
});
BindToOption(EnableDocumentOutline, DocumentOutlineOptionsStorage.EnableDocumentOutline);
}

// Since this dialog is constructed once for the lifetime of the application and VS Theme can be changed after the application has started,
Expand Down
4 changes: 1 addition & 3 deletions src/VisualStudio/Core/Def/DocumentOutline/DocumentOutlineOptionsStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ namespace Microsoft.VisualStudio.LanguageServices.DocumentOutline;
/// </summary>
internal sealed class DocumentOutlineOptionsStorage
{
public static readonly Option2<bool?> EnableDocumentOutline = new("visual_studio_enable_document_outline", defaultValue: null);

public static readonly Option2<bool> DisableDocumentOutlineFeatureFlag = new("visual_studio_disable_document_outline_feature_flag", defaultValue: false);
public static readonly Option2<bool> EnableDocumentOutline = new("visual_studio_enable_document_outline", defaultValue: true);

public static readonly Option2<SortOption> DocumentOutlineSortOrder = new("visual_studio_document_outline_sort_order", defaultValue: SortOption.Location);
}
3 changes: 1 addition & 2 deletions src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ private void GetOutline(out IntPtr phwnd)
{
phwnd = default;

var enabled = _globalOptions.GetOption(DocumentOutlineOptionsStorage.EnableDocumentOutline)
?? !_globalOptions.GetOption(DocumentOutlineOptionsStorage.DisableDocumentOutlineFeatureFlag);
var enabled = _globalOptions.GetOption(DocumentOutlineOptionsStorage.EnableDocumentOutline);
if (!enabled)
return;

Expand Down
3 changes: 0 additions & 3 deletions src/VisualStudio/Core/Def/Options/VisualStudioOptionStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ public bool TryFetch(LocalUserRegistryOptionPersister persister, OptionKey2 opti
{"dotnet_preview_inline_rename_changes", new RoamingProfileStorage("TextEditor.Specific.PreviewRename")},
{"dotnet_collapse_suggestions_in_inline_rename_ui", new RoamingProfileStorage("TextEditor.CollapseRenameSuggestionsUI")},
{"dotnet_commit_rename_asynchronously", new RoamingProfileStorage("TextEditor.Specific.CommitRenameAsynchronously")},
{"dotnet_commit_rename_asynchronously_feature_flag", new FeatureFlagStorage("Roslyn.CommitRenameAsynchronously")},
{"dotnet_rename_get_suggestions_automatically", new FeatureFlagStorage(@"Editor.AutoSmartRenameSuggestions")},
// Option is deprecated, don't use the same RoamingProfileStorage key
// {"dotnet_rename_asynchronously", new RoamingProfileStorage("TextEditor.Specific.RenameAsynchronously")},
Expand Down Expand Up @@ -401,7 +400,6 @@ public bool TryFetch(LocalUserRegistryOptionPersister persister, OptionKey2 opti
{"csharp_split_string_literal_on_return", new RoamingProfileStorage("TextEditor.CSharp.Specific.SplitStringLiterals")},
{"visual_studio_open_stack_trace_explorer_on_focus", new RoamingProfileStorage("StackTraceExplorer.Options.OpenOnFocus")},
{"visual_studio_enable_document_outline", new RoamingProfileStorage(@"DocumentOutline.Enable")},
{"visual_studio_disable_document_outline_feature_flag", new FeatureFlagStorage(@"DocumentOutline.DisableFeature")},
{"visual_studio_document_outline_sort_order", new RoamingProfileStorage(@"DocumentOutline.SortOrder")},
{"visual_studio_enable_symbol_search", new LocalUserProfileStorage(@"Roslyn\Features\SymbolSearch", "Enabled")},
{"dotnet_unsupported_search_nuget_packages", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.SuggestForTypesInNuGetPackages")},
Expand All @@ -422,7 +420,6 @@ public bool TryFetch(LocalUserRegistryOptionPersister persister, OptionKey2 opti
{"visual_studio_navigate_to_object_browser", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.NavigateToObjectBrowser")},
{"dotnet_validate_compilation_tracker_states", new FeatureFlagStorage(@"Roslyn.ValidateCompilationTrackerStates")},
{"dotnet_enable_diagnostics_in_source_generated_files", new RoamingProfileStorage("TextEditor.Roslyn.Specific.EnableDiagnosticsInSourceGeneratedFilesExperiment")},
{"dotnet_enable_diagnostics_in_source_generated_files_feature_flag", new FeatureFlagStorage(@"Roslyn.EnableDiagnosticsInSourceGeneratedFiles")},
{"dotnet_source_generator_execution", new RoamingProfileStorage("TextEditor.Roslyn.Specific.SourceGeneratorExecution")},
{"dotnet_reload_changed_analyzer_references", new LocalUserProfileStorage(@"Roslyn\Internal\OnOff\Features", "DotnetReloadChangedAnalyzerReferences")},
{"xaml_enable_lsp_intellisense", new FeatureFlagStorage(@"Xaml.EnableLspIntelliSense")},
Expand Down
18 changes: 3 additions & 15 deletions src/VisualStudio/VisualBasic/Impl/Options/AdvancedOptionPageControl.xaml.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Options
BindToOption(Automatic_Run_generators_after_any_change, WorkspaceConfigurationOptionsStorage.SourceGeneratorExecution, SourceGeneratorExecutionPreference.Automatic)
BindToOption(Balanced_Run_generators_after_saving_or_building, WorkspaceConfigurationOptionsStorage.SourceGeneratorExecution, SourceGeneratorExecutionPreference.Balanced)

BindToOption(Analyze_source_generated_files, SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFiles,
Function()
' If the option has not been set by the user, check if the option is enabled from experimentation.
Return optionStore.GetOption(SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFilesFeatureFlag)
End Function)
BindToOption(Analyze_source_generated_files, SolutionCrawlerOptionsStorage.EnableDiagnosticsInSourceGeneratedFiles)

' Rename
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.CommitRenameAsynchronously,
Function()
Return optionStore.GetOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronouslyFeatureFlag)
End Function)
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.CommitRenameAsynchronously)
BindToOption(Rename_UI_setting, InlineRenameUIOptionsStorage.UseInlineAdornment, label:=Rename_UI_setting_label)

' Import directives
Expand Down Expand Up @@ -174,12 +167,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Options
BindToOption(IncludeGlobalImports, InheritanceMarginOptionsStorage.InheritanceMarginIncludeGlobalImports, LanguageNames.VisualBasic)

' Document Outline
BindToOption(EnableDocumentOutline, DocumentOutlineOptionsStorage.EnableDocumentOutline,
Function()
' If the option has Not been set by the user, check if the option is disabled from experimentation.
' If so, default to reflect that.
Return Not optionStore.GetOption(DocumentOutlineOptionsStorage.DisableDocumentOutlineFeatureFlag)
End Function)
BindToOption(EnableDocumentOutline, DocumentOutlineOptionsStorage.EnableDocumentOutline)
End Sub

' Since this dialog is constructed once for the lifetime of the application and VS Theme can be changed after the application has started,
Expand Down
Loading