Add .editorconfig file #404
Merged
+4,343
−3,285
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sergio0694
added
help wanted 🆘
Nirmal4G
reviewed
Oct 13, 2022
There was a problem hiding this comment.
If I open the solution and build and intentionally write code that doesn't respect the
.editorconfigfile (eg. a local usingvar), I get no warnings at all 🤔
Could be just a cache issue on Compiler side. I don't have the repo cloned. And I'm replying from mobile. I'll take a look at it soon.
|
I don't think it's VS caching, as I've also tried on different machines and VS versions, and I never got the warnings. I'm not sure what's different than in the Toolkit repo 🤔 |
Sergio0694
force-pushed
the
dev/editorconfig
branch
from
6414615 to
7a11cc9
Compare
Nirmal4G
suggested changes
Oct 14, 2022
There was a problem hiding this comment.
So, there are 3 issues at play here:
-
XAML files placed inside C# files, leading to dotnet rules not applying to C# files.
-
Code style analyzer does not match the compiler's version leading to
S.C.Iversion mismatch. -
An unrelated issue but a warning for missing Appx bundling platforms appears for a sample UWP project.
This patch fixes all of that: 18e2a7e
But I can't push to the PR. You can get my patch from my fork...
git switch dev/editorconfig
git pull --ff-only https://github.com/Nirmal4G/ComputeSharp dev/editorconfig
git push
|
Ooh, thank you! Yeah using the explicit Roslyn version is annoying, unfortunately we're stuck with that until VS 17.4 is out 🥲 |
|
Hey @Nirmal4G, so it seems .editorconfig is working now and I did get a bunch of errors, which I fixed in b9c1f2c. I still have a few questions though and there's some things I'm seeing that I don't understand, would you be able to help?
Thank you! 😊 |
Sergio0694
force-pushed
the
dev/editorconfig
branch
from
1a76c8d to
bd4e624
Compare
Nirmal4G
suggested changes
Oct 14, 2022
There was a problem hiding this comment.
Running dotnet format in the context of an MSBuild solution lead to miss some files that were not included in the solution and its projects. You should run with --folder for the 2nd time to verify.
Here's the patch with missing files formatted: 1370019
It's in my branch, so same process as before.
It might be due to differences in MSBuild build, VS DesignTime build and a VS regular build. Yes, they differ subtly. Most of the time It should work the same across different build contexts. I'll look into that. |
Thank you! Yeah, I'd really like for all of those warnings to always show up so I can properly enforce them everywhere 🙂 |
Youssef1313
reviewed
Oct 14, 2022
Youssef1313
reviewed
Oct 14, 2022
Youssef1313
reviewed
Oct 14, 2022
Youssef1313
reviewed
Oct 14, 2022
Sergio0694
force-pushed
the
dev/editorconfig
branch
from
a9ebe98 to
e161e34
Compare
|
@Youssef1313 I've updated my .editorconfig with some better rules (eg. I fixed the fields naming convention), but if I run Note: it's not just about it missing some files, it's also not applying all fixes. Eg. it's not renaming fields correctly. If I open the solution in VS after running it, I still see a bunch of IntelliSense errors for fields named with the wrong style. |
|
@Sergio0694 What errors about "loading some projects correctly" do you see? |
|
@Youssef1313 I get all of this 😥 `dotnet format` output (click to expand): |
Sergio0694
force-pushed
the
dev/editorconfig
branch
from
7b2bdd0 to
e053617
Compare
|
I've now enabled all analyzers (just disabled a few I didn't like), and fixed all code to produce 0 warnings 🎉 @Youssef1313 if you take another look let me know if everything looks fine to you! 😄 |
Youssef1313
reviewed
Oct 15, 2022
Youssef1313
reviewed
Oct 15, 2022
Sergio0694
force-pushed
the
dev/editorconfig
branch
from
765321d to
026e671
Compare
There are 3 issues at play here: 1. XAML files placed inside C# files, leading to dotnet rules not applying to C# files. 2. Code style analyzer not matching the compiler's version. 3. Unrelated but warning for missing Appx bundling platforms appears for a sample UWP project. This patch fixes all of that.
Sergio0694
force-pushed
the
dev/editorconfig
branch
from
026e671 to
e0a8c29
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR ports the .editorconfig file from the .NET Community Toolkit (https://github.com/CommunityToolkit/dotnet).
@Nirmal4G this seems like your cup of team, any ideas what might be wrong with this? Could use some help 😄