Skip to content

Prevent assert from being hit#62366

Merged
davidwengier merged 2 commits intodotnet:mainfrom
davidwengier:DiagnosticsWindowOptions
Jul 7, 2022
Merged

Prevent assert from being hit#62366
davidwengier merged 2 commits intodotnet:mainfrom
davidwengier:DiagnosticsWindowOptions

Conversation

@davidwengier
Copy link
Member

Fixes #62365

@davidwengier davidwengier requested a review from a team as a code owner July 5, 2022 07:22
@ghost ghost added the Area-IDE label Jul 5, 2022
panel.Children.Add(label);
panel.Children.Add(textBox);

AddSearchHandler(label);
Copy link
Member

@Youssef1313 Youssef1313 Jul 5, 2022

Choose a reason for hiding this comment

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

A small suggestion (doesn't need to be in this PR):

Is it possible to do a change similar to the following:

// see https://github.com/dotnet/roslyn/blob/8fd033cd948a2eb33e4af650b7a1b3dd18e914d2/src/VisualStudio/Core/Impl/Options/AbstractOptionPage.cs#L24
// renamed the abstract CreateOptionPage to CreateOptionPageCore, this is what needs to be overridden now.
protected abstract AbstractOptionPageControl CreateOptionPageCore(IServiceProvider serviceProvider, OptionStore optionStore);


protected AbstractOptionPageControl CreateOptionPage(IServiceProvider serviceProvider, OptionStore optionStore)
{
    var optionPage = CreateOptionPageCore();
    // loop over all labels, comboboxes, checkboxes and call AddSearchHandler
    return optionPage;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

If this were winforms, I'd absolutely do that.. but its not, and I don't think WPF has a convenient way of iterating child controls, across the myriad of options for containers and stack layouts and grids etc.

Plus, 99% of the search handler setup is already automatic because most things just call the base Bind... method anyway, so I don't think this would save much.

if (option.Type == typeof(bool))
{
var checkBox = new CheckBox() { Content = option.Name + GetLanguage(languageName) + GetAdditionalText(additional) };
var checkBox = new CheckBox() { Content = option.Name.Replace('_', ' ') + GetLanguage(languageName) + GetAdditionalText(additional) };
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this needs a comment here...

@davidwengier davidwengier merged commit 0be968a into dotnet:main Jul 7, 2022
@davidwengier davidwengier deleted the DiagnosticsWindowOptions branch July 7, 2022 21:37
@ghost ghost added this to the Next milestone Jul 7, 2022
333fred added a commit to 333fred/roslyn that referenced this pull request Jul 7, 2022
* upstream/main: (62 commits)
  Prevent assert from being hit (dotnet#62366)
  Don't offer '??=' for pointer types (dotnet#62476)
  Integrate generator times into /reportAnalyzer (dotnet#61661)
  Switch to a callback for cleaning up resources instead of passing in an explicit IDisposable. (dotnet#62373)
  Filter trees to only those containing global-usings or attributes prior to analyzing them. (dotnet#62444)
  Update PublishData.json
  Complete 'file' support for `SyntaxGenerator` (dotnet#62413)
  Apply changes directly to text buffer (dotnet#62337)
  Remove LangVer check from extended nameof binding (dotnet#62339)
  Fixed shared project file error (dotnet#62466)
  Handle new error codes
  Use MSBuid generated property for package path
  Exclude BCL libraries from Roslyn vsix
  Bump the integration test timeouts a bit
  Skip the balanced switch dispatch optimization for patterns on floating-point inputs (dotnet#62322)
  Test helpers shouldn't escape quotes by default (dotnet#62321)
  Reuse prior TableEntry values in the increment NodeTable builder if possible. (dotnet#62320)
  Install 3.1 runtime for SBOM tool
  Generate VS SBOM during official build.
  Minor refactoring in 'required' handling for override completion (dotnet#62422)
  ...
@allisonchou allisonchou modified the milestones: Next, 17.4 P1 Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

search handler assertion failure

4 participants