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

Add UI for editorconfig files #51069

Merged
25 commits merged into from
Mar 29, 2021
Merged

Add UI for editorconfig files #51069

25 commits merged into from
Mar 29, 2021

Conversation

jmarolf
Copy link
Contributor

@jmarolf jmarolf commented Feb 8, 2021

This is the new UI that will appear when a developer opens an editorconfig file (note a user can still get to the old file view via "Open With..")

Feature walkthrough:

  • Searching

search

  • Filtering

filter

  • Sorting

sort

  • Saving

saving

Formatting Tab

image

Code Style Tab

image

Analyzers Tab

image

TODO:

  • Finish adding tests for editorconfig saving helpers
  • Update Tools -> Options window to use new services
  • Unify editorconfig saving types
  • Remove/fix all //TODO(jmarolf) comments

@jmarolf jmarolf requested a review from a team as a code owner February 8, 2021 16:44
Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

nit: typo

@mavasani
Copy link
Contributor

mavasani commented Feb 8, 2021

@jmarolf Do you have UI screenshots?

@CyrusNajmabadi
Copy link
Member

Pictures plz @jmarolf .

1 similar comment
@CyrusNajmabadi
Copy link
Member

Pictures plz @jmarolf .

@jmarolf jmarolf marked this pull request as draft February 17, 2021 03:00
@jmarolf
Copy link
Contributor Author

jmarolf commented Feb 18, 2021

@CyrusNajmabadi I pictured. Still working to update some things.

@jmarolf jmarolf force-pushed the feature/settings-ui branch 2 times, most recently from 4b8b633 to a76c077 Compare February 22, 2021 18:01
@jmarolf
Copy link
Contributor Author

jmarolf commented Feb 22, 2021

@CyrusNajmabadi would appreciate your opinion here. I believe I will be able to unify on IOption2 type for codestyle and whitespace settings so those additional types will likely go away soon, but are there any other concerns about the approach taken here?

@jmarolf jmarolf force-pushed the feature/settings-ui branch 2 times, most recently from 62d56db to 1780576 Compare February 26, 2021 16:04
@jmarolf jmarolf changed the title [WIP] Add UI for editorconfig files Add UI for editorconfig files Feb 26, 2021
@jmarolf jmarolf removed request for a team March 29, 2021 06:29
@ghost ghost merged commit f52afd9 into dotnet:main Mar 29, 2021
@ghost ghost added this to the Next milestone Mar 29, 2021
@JohanLarsson
Copy link

One thing that would be useful is a checkbox for disabling all rules in a group. One use case is when disabling all StyleCop documentation rules in a test project.

@vatsalyaagrawal vatsalyaagrawal modified the milestones: Next, 16.10.P2 Mar 29, 2021
@vatsalyaagrawal
Copy link
Contributor

@jmarolf Yay!! Looking forward to shipping this. 🎉🎉

@jnm2
Copy link
Contributor

jnm2 commented Apr 2, 2021

One thing that would be useful is a checkbox for disabling all rules in a group. One use case is when disabling all StyleCop documentation rules in a test project.

I would expect it to be a single line disabling the category when possible, like with this StyleCop example.

@jmarolf
Copy link
Contributor Author

jmarolf commented Apr 3, 2021

@jnm2 got it: tracking here: #52388


namespace Microsoft.CodeAnalysis.Editor.EditorConfigSettings.Updater
{
internal static partial class SettingsUpdateHelper
Copy link
Contributor

Choose a reason for hiding this comment

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

@jmarolf This file should not have been added. It duplicates a lot of functionality from ConfigurationUpdater used in Configure Severity lightbulb action, and leads to many inconsistencies between how editorconfig is updated under the covers from Editorconfig UX versus Configure Severity lightbulb action (#52720). Latter also handles updating category based severities, which I believe is a feature request for Editorconfig UX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me file a bug to explicitly track this. The intention was that I would send a cleanup PR to unify these implementations. As an aside: are there any tests for ConfigurationUpdater? I couldn't find any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@Youssef1313 Youssef1313 Apr 19, 2021

Choose a reason for hiding this comment

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

@jmarolf Not directly tested, but the tests in here (VB) and here (C#) test it indirectly (they test the CodeFixProvider which depends on ConfigurationUpdater)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, thank you

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet