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

Remove trim_whitespaces from editorconfig #40884

Closed
wants to merge 1 commit into from

Conversation

Youssef1313
Copy link
Member

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Aug 15, 2020

Tagging subscribers to this area: @ViktorHofer
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member

jkotas commented Aug 15, 2020

Did you find this option problematic in dotnet/runtime repo?

I have not run into any of the issues mentioned in the linked comment while working in dotnet/runtime.

@jkotas
Copy link
Member

jkotas commented Aug 15, 2020

Maybe there are a few files where it may be causing problem, but I would think that the benefits outweighs it.

@stephentoub
Copy link
Member

I don't understand why we're removing this. It's very helpful.

@jkotas jkotas closed this Aug 15, 2020
@Youssef1313
Copy link
Member Author

@stephentoub @jkotas Thanks for reviews. But as stated in sharwell's comment:

Some files have significant trailing whitespace (e.g. Markdown, C# verbatim string literals)

I think if, now there aren't significant trailing whitespaces, I think it's not guaranteed that this will remain the same in future.

@stephentoub
Copy link
Member

stephentoub commented Aug 15, 2020

We have an analyzer that fails the build if there's trailing whitespace in C# files.

And end-of-line literally invisible code is practically impossible to maintain.

Please point to an actual example in dotnet/runtime where it's causing a problem.

@Youssef1313
Copy link
Member Author

@stephentoub Thanks for clarifying that. In that case, it would be possibly to exclude this option for markdown only? (I haven't seen an example in this repo where markdown trailing spaces are significant, but I've seen that in dotnet/docs before).

Also, can I ask which analyzer is used for the trailing whitespaces in C# files?

@Youssef1313
Copy link
Member Author

@stephentoub Another solution for the markdown files is to enable markdownlint with MD009 rule enabled.

monojenkins pushed a commit to monojenkins/mono that referenced this pull request Aug 20, 2020
akoeplinger pushed a commit to mono/mono that referenced this pull request Aug 20, 2020
Related to dotnet/runtime#40887 and dotnet/runtime#40884

Co-authored-by: Youssef1313 <Youssef1313@users.noreply.github.com>
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants