Skip to content

Clean up the IntelliSense option pages#71777

Merged
Cosifne merged 11 commits intodotnet:mainfrom
Cosifne:dev/shech/cleanUpIntellisensePage
Jan 24, 2024
Merged

Clean up the IntelliSense option pages#71777
Cosifne merged 11 commits intodotnet:mainfrom
Cosifne:dev/shech/cleanUpIntellisensePage

Conversation

@Cosifne
Copy link
Member

@Cosifne Cosifne commented Jan 23, 2024

I am going to move the IntelliSense page to VS new Unified Settings.
Our old page would still be used because the new Unified Settings is in preview.
Before that happens, I would like to clean up our Intellisense page because it's in a strange stage now.
See my comments for detailed changes.

@Cosifne Cosifne requested a review from a team as a code owner January 23, 2024 19:26
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 23, 2024
Unchecked="Show_items_from_unimported_namespaces_CheckedChanged"
Indeterminate="Show_items_from_unimported_namespaces_CheckedChanged"
IsThreeState="True"/>
Content="{x:Static local:IntelliSenseOptionPageStrings.Option_Show_items_from_unimported_namespaces}" />
Copy link
Member Author

@Cosifne Cosifne Jan 23, 2024

Choose a reason for hiding this comment

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

Show_items_from_unimported_namespaces, Tab_twice_to_insert_arguments and Show_new_snippet_experience
are using the three state checkbox tricks to indicate they are in experiment.
However, I found

  1. Show_items_from_unimported_namespaces is on by default. I can't find its feature flag.
  2. Tab_twice_to_insert_arguments is off by default now. I also can't find the feature flag. I synced with Sam and he intend to keep it off.
  3. Show_new_snippet_experience has the feature flag. But it no longer need to use the three state trick because the value could be fetched from the feature flag when it's null.

InitializeComponent();

BindToOption(Automatically_complete_statement_on_semicolon, CompleteStatementOptionsStorage.AutomaticallyCompleteStatementOnSemicolon);
BindToOption(Show_completion_list_after_a_character_is_typed, CompletionOptionsStorage.TriggerOnTypingLetters, LanguageNames.CSharp);
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 reorder the sequence here to match the order in xaml.
image


Show_new_snippet_experience.IsChecked = this.OptionStore.GetOption(CompletionOptionsStorage.ShowNewSnippetExperienceUserOption, LanguageNames.CSharp);
AddSearchHandler(Show_new_snippet_experience);
BindToOption(Tab_twice_to_insert_arguments, CompletionViewOptionsStorage.EnableArgumentCompletionSnippets, LanguageNames.CSharp, onNullValue: () => false);
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no feature flag for this option. So now it's off by default. And I have synced with Sam he is OK to leave this off

Show_completion_list_after_a_character_is_deleted_Unchecked(sender, e);
}

private void Show_completion_list_after_a_character_is_deleted_Checked(object sender, RoutedEventArgs e)
Copy link
Member Author

Choose a reason for hiding this comment

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

Show_completion_list_after_a_character_is_deleted_Checked and Show_completion_list_after_a_character_is_deleted_Unchecked could be removed because on line 22
BindToOption(Show_completion_list_after_a_character_is_deleted, CompletionOptionsStorage.TriggerOnDeletion, LanguageNames.CSharp, onNullValue: () => false);

private void Show_completion_list_after_a_character_is_deleted_Unchecked(object sender, RoutedEventArgs e)
=> this.OptionStore.SetOption(CompletionOptionsStorage.TriggerOnDeletion, LanguageNames.CSharp, value: false);

private void Show_items_from_unimported_namespaces_CheckedChanged(object sender, RoutedEventArgs e)
Copy link
Member Author

Choose a reason for hiding this comment

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

On line 41, BindToOption(Show_items_from_unimported_namespaces, CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, onNullValue: static () => true);, so we don't need the three state trick

this.OptionStore.SetOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, value: Show_items_from_unimported_namespaces.IsChecked);
}

private void Tab_twice_to_insert_arguments_CheckedChanged(object sender, RoutedEventArgs e)
Copy link
Member Author

Choose a reason for hiding this comment

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

On line 43 BindToOption(Tab_twice_to_insert_arguments, CompletionViewOptionsStorage.EnableArgumentCompletionSnippets, LanguageNames.CSharp, onNullValue: static () => false); , so these methods could be removed

BindToOption(Show_completion_list_after_a_character_is_typed, CompletionOptionsStorage.TriggerOnTypingLetters, LanguageNames.VisualBasic)
Show_completion_list_after_a_character_is_deleted.IsChecked = Me.OptionStore.GetOption(
CompletionOptionsStorage.TriggerOnDeletion, LanguageNames.VisualBasic) <> False
BindToOption(Show_completion_list_after_a_character_is_deleted, CompletionOptionsStorage.TriggerOnDeletion, LanguageNames.VisualBasic, function() True)
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: VB's behavior is not aligned with C# here.
In C# the original code is Show_completion_list_after_a_character_is_deleted.IsChecked = this.OptionStore.GetOption(CompletionOptionsStorage.TriggerOnDeletion, LanguageNames.CSharp) == true;, which means the checkbox is false when option value is null.

But in VB (see the old code), the checkbox is true, when option value is null.
I just keep this behavior.

Copy link
Member Author

@Cosifne Cosifne Jan 23, 2024

Choose a reason for hiding this comment

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

Also, VB's behavior is different from C# here.
When CompletionOptionsStorage.TriggerOnTypingLetters is off, in VB, we don't set CompletionOptionsStorage.TriggerOnDeletion to off. But in C# we do.
Not sure what's the original user story. I just keep this

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

:shipit:

@Cosifne Cosifne merged commit 8ce4351 into dotnet:main Jan 24, 2024
@Cosifne Cosifne deleted the dev/shech/cleanUpIntellisensePage branch January 24, 2024 18:40
@ghost ghost added this to the Next milestone Jan 24, 2024
@Cosifne Cosifne modified the milestones: Next, 17.10 P1 Jan 30, 2024
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.

2 participants