-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Enable new inline rename by default #62992
Conversation
Should there also be a VB.NET setting for this? |
This is where we get a little tricky. I can duplicate in VB, but it only applies globally not per language. I guess we could do per language... Unfortunately we don't have a ".NET" node to put stuff like this in :( |
@@ -83,6 +89,16 @@ | |||
<StackPanel> | |||
<CheckBox x:Name="Rename_asynchronously_exerimental" | |||
Content="{x:Static local:AdvancedOptionPageStrings.Option_Rename_asynchronously_experimental}" /> | |||
|
|||
<Label x:Name="Rename_UI_setting_label" Content="{x:Static local:AdvancedOptionPageStrings.Where_should_the_rename_ui_be_shown}" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bit odd that ui
is lowercased.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just Rename textbox location: inline/in-top-right
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not completely accurate though. It's a bit more nuanced than that. At most, inline rename options or dashboard them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put a screenshot in of the options, with the dropdown dropped? Seems like the options are a bit too wordy, and repeat "UI" a lot, but hard to tell for sure from just resource strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be added to AutomationObject
?
I'm concerned by this being added both to C#-specific and VB-specific options page but have a single value for both (ie, if user modified it in one language, it gets automatically the same value in the other).
Are there other similar options that are single-valued for both languages and exposed in Tools → Options?
Ideally there should be some sort of a "common" page. @mavasani What do you think?
Allow navigation to decompiled sources is for both languages, and only appears on the C# page, presumably because there is nowhere else to put it. |
I haven't kept up with |
@ryzngard I don't know of docs about it, but basically you can observe the impact by:
When importing the file back, the old value should be seen. |
Editor Color Scheme falls in this bucket. Both pages bind to the same option (not perlanguage) and the OptionStore class ensures that changes are synced. |
I don't think there should be a common option page for these cases where the same option applies to both C# and VB and its value is kept in sync by the OptionStore infrastructure. That would be confusing to users who only work with a single language and want to look at all applicable options for the language. IMO common option pages should only be defined for language-agnostic options and options that apply to all languages supported in VS, and that is the core reason why Roslyn repo has no such common option page infrastructure, we only deal with C# and VB options. Common option page infrastructure is present in the VS platform layer which supports such language-agnostic options. |
…th new experience
No description provided.