dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.88k stars 4.01k forks source link

Add extension types to default .editorconfig templates #47787

Open timheuer opened 3 years ago

timheuer commented 3 years ago

In the .editorconfig templates one can add through Visual Studio, the file types registered for the tab spacing are:

indent_style = tab
# Code files
[*.{cs,csx,vb,vbx}]
indent_size = 4
insert_final_newline = true
charset = utf-8-bom

Presently in Visual Studio, the XML editor defaults are tabs, not spaces like all other areas. EditorConfig is some guidance we use for consistency and we should add common .NET XML file types to our default template. Namely change to add csproj/vbproj/fsproj/xml as types so the new version would be something like this.

indent_style = tab
# Code files
[*.{cs,csx,vb,vbx,csproj,vbproj,fsproj,xml}]
indent_size = 4
insert_final_newline = true
charset = utf-8-bom
sharwell commented 3 years ago

@timheuer There are several different ways to create a .editorconfig file, and they are owned by several different teams. Can you include a screenshot showing how you are creating the file so we can get this issue assigned correctly?

davkean commented 3 years ago

And .props/.targets.

jmarolf commented 3 years ago

Presently in Visual Studio, the XML editor defaults are tabs, not spaces like all other areas. EditorConfig is some guidance we use for consistency and we should add common .NET XML file types to our default template. Namely change to add csproj/vbproj/fsproj/xml as types so the new version would be something like this.

Personally, agree with this. Would like us to at least have the editorconfig files we generate support spaces as that is the default for .NET code files today.

There are several different ways to create a .editorconfig file, and they are owned by several different teams. Can you include a screenshot showing how you are creating the file so we can get this issue assigned correctly?

All the different teams are creating editorcondig files in accordance with how we've advised them so I don't think we need more info to make a call here on whether this is the right thing to do.

they are owned by several different teams.

Would not be a lot of work to update all of them to support this if we wanted. I still think the .NET Tools group (us) should make that call on what the defaults for .NET should be. These other editorconfig impls should follow whatever desgin we set out.

CyrusNajmabadi commented 3 years ago

I think we can only make decisions on the xml experience by discussing this with the xml team. Not sure how is best tehre. But maybe @mgoertz-msft has an idea? It shouldn't be Roslyn (or intellicode) making this decision for them.

mgoertz-msft commented 3 years ago

Plain XML is not actually owned by our team. Interestingly, if I reset my VS settings to either General or C# I end up with TabSize of 4 and using tabs but what I had before used to be 2 spaces. Not sure where that came from but the .editorconfig template here also has a TabSize of 2.

Anyway, I would have to find the owning team but I'm not sure that they will have a strong opinion since the XML editor is pretty much in maintenance mode.

And for XAML, it would make sense to add it to that list too since 4 spaces matches the default anyway.

timheuer commented 3 years ago

@mgoertz-msft - what I'm asking here is to change the templates that are the .NET team. The defaults from VS are being looked at by the XML editor team.

genlu commented 3 years ago

Design meeting note:

mgoertz-msft commented 3 years ago

@timheuer Yeah, I get that. But since .editorconfig settings take precedence over VS defaults it could still be confusing to some users if their defaults for other languages change once they add a new .editorconfig to a folder.

timheuer commented 3 years ago

@mgoertz-msft but that's no diff than the overriding we would do for the other ones (e.g., I could have changed my C# to tabs and this editorconfig changes my defaults). The goal here is that once a .NET template is added that the most used types for that project follow the same.

mgoertz-msft commented 3 years ago

@timheuer Sure, but I would argue that for the best user experience the default template (not the dynamically generated one obviously) should be in sync with the defaults and not competing with them. It's a discussion that should be had with the XML team.

For XAML on the other hand it's all groovy. :-)

jmarolf commented 3 years ago

But since .editorconfig settings take precedence over VS defaults it could still be confusing to some users if their defaults for other languages change once they add a new .editorconfig to a folde

This is why we are only going to change csproj and vbproj files by default. all other filetypes could have multiple purposes but we know these files are only about .NET tooling

The proposal is to change what we generate for editorconfig files from this:

root = true
# All files
[*]
indent_style = space
# Code files
[*.{cs,csx,vb,vbx}]
indent_size = 4
insert_final_newline = true
charset = utf-8-bom

to this:

root = true
# All files
[*]
indent_style = space
# Project Files
[*.{csproj,vbproj}]
indent_size = 2
# Code files
[*.{cs,csx,vb,vbx}]
indent_size = 4
# Project and Code Files
[*.{cs,csx,csproj,vb,vbx,vbproj}]
insert_final_newline = true
charset = utf-8-bom

This matches the spacing that is used today in csproj files when they are created via the .NET CLI or Visual Studio.

@timheuer any objections?

timheuer commented 3 years ago

@jmarolf no objections on the isolated change. I'm already working with XML editor on their VS defaults.

davkean commented 3 years ago

It should respect options, no?

jmarolf commented 3 years ago

If the XML editor doesn't respect the setting then thats definitely a bug.

davkean commented 3 years ago

No, I mean I've told XML files to have 4 spaces in options, and you're explicitly setting it to 2.

jmarolf commented 3 years ago

ah, well my intuition is that for the template case the user is saying "Override my defaults with whatever is recommended" in that case I am ok with setting two spaces even the user has different VS settings.

For the other two cases 1.) intellicode 2.) generating and editorconfig file from existing settings you are correct. The User just wants an editorconfig file that best represents their current settings. In this case we should just output whatever the VS Settings are.

perhaps the intellicode inference could do more advanced things but this seems a reasonable place to start.