Skip to content
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

VSMac: Make QuickFix preview resizable and add title #49394

Merged
merged 1 commit into from
Nov 23, 2020

Conversation

DavidKarlas
Copy link
Contributor

This PR makes few improvements to QuickFix preview to fit nicely in newer QuickFix design
Also worked around missing IPreviewDialogService


#pragma warning disable IDE0052 // Remove unread private members
#pragma warning disable IDE0060 //Remove unused parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Which parameter is this for? Is it an analyzer false positive, or a valid complaint? It looks like everything is used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In constructor optionPageGuid, logIdVerbatimInTelemetry...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before they were set to field but not anymore... Hence different warning...

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, yes of course. Do we need to keep them around, or are you just avoiding breaking VS for Mac?

I'm asking because of #48871 - we'd like to clean all of these up if we can, slowly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its called from this project so it wouldn't break, its just that goal is to use all this in future, and removing it would need to delete code on PreviewPaneService.cs that we would have to re-add later...

This PR makes few improvements to QuickFix preview to fit nicely in newer QuickFix design
Also worked around missing `IPreviewDialogService`
@DavidKarlas DavidKarlas force-pushed the dev/davkar/quickFixPreview branch from ea4f27f to 0e9350a Compare November 20, 2020 06:40
@davidwengier davidwengier merged commit 2834b74 into dotnet:master Nov 23, 2020
@ghost ghost added this to the Next milestone Nov 23, 2020
@allisonchou allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020
333fred added a commit to 333fred/roslyn that referenced this pull request Nov 30, 2020
* upstream/master: (265 commits)
  Use extra generic type parameters and apply C#-specific knowledge to all langs instead of using inheritance
  Cover all changed code paths
  Stop removing parens that are required by C#
  Fix unnecessary spans
  Failing test for preserving parens around conditional expression
  AddSynthesizedRecordMembersIfNecessary - avoid touching members that are known to have no effect on the outcome of the function. (dotnet#49610)
  Resolve follow-up comments in PR "Create default arguments during binding" (dotnet#49588)
  Remove restore and checkout from test jobs (dotnet#49452)
  3.8.* -> 3.9.*
  Update PublishData.json
  Update Versions.props
  Remove Microsoft.CodeAnalysis.VisualBasic.dll from the VSPE.OptProfTests.DDRIT_RPS_ManagedLangs_Typing runs
  Fix the ability to expand the list of analyzers in a reference
  Fix comment
  Address feedback to ensure `/warnaserror-:ID` prevents config options from bumping a warning to an error.
  parallel restore on mac/linux (dotnet#49523)
  VSMac: Make QuickFix preview resizable and add title (dotnet#49394)
  Add CallerMemberNameAttributeWithImplicitObjectCreation test (dotnet#49556)
  Update dependencies from https://github.com/dotnet/arcade build 20201120.10 (dotnet#49541)
  Clarify comment
  ...
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.

5 participants