Roslyn: Support for .editorconfig csharp_trim_trailing_whitespace

[jnm2]

Trimming trailing whitespace has been particularly important for me any time git diffs are involved. Depending on the codebase you're working in, you either really want this forced on or really want this forced off.

The problem is that trim_trailing_whitespace is not C#-safe. As far as I know the only possible conflict is with multiline verbatim strings.

I really need a C#-safe trim-on-save for some codebases. Could we have a csharp_trim_trailing_whitespace?

Even better, there should probably even be an editor warning if you're using trim_trailing_whitespace and a suggestion to switch to csharp_trim_trailing_whitespace.

[tannergooding]

I wonder if this impacts doc comments as well...

[jasonmalinowski]

Is there any reason we wouldn't just make trim_trailing_whitespace do The Right Thing(tm)? We'd need some hook with the editor for it to ask whether trailing whitespace is trimmable, but that's pretty easy to implement. My worry is people are going to go to editorconfig.org, read that the base option exists, and then turn that on and never realize they had something they should do instead.

Counter-argument to this is it presumes other editors will also do The Right Thing(tm) too -- your VS project work fine, but a different editor will see the default option and do the wrong thing and that's still bad.

FYI to @olegtk.

[Pilchie]

@sharwell pointed out in the other issue, the problem with the normal trim_trailing_whitespace is that even if we support it properly, most editors won't, so it might be nicer to have trim_trailing_whitespace false, but set csharp_trim_trailing_whitespace to true so that editors that know how to do the right thing do it, and ones that don't don't break things.

[jasonmalinowski]

I wonder if it's almost worth allowing trim_trailing_whitespace = false/true/safe. And push for an update to the core spec. Or maybe it just needs a doc in the core docs to say "this option exists, you might not want to use it?"

[tannergooding]

@jasonmalinowski, Just wondering, but how would you propose the core spec specify that 'safe' is handled?

[jasonmalinowski]

Probably something like:

"safe" means an editor SHOULD NOT remove trailing whitespace unless the editor understands that such removal is safe per the semantics of the file.

Really comes down to wondering whether most existing .editorconfig implementations handled the string to Boolean conversion by comparing equal to "true" or comparing not equal to "false".

[jnm2]

Really comes down to wondering whether most existing .editorconfig implementations handled the string to Boolean conversion by comparing equal to "true" or comparing not equal to "false".

I would hope that they would completely ignore any line that isn't parseable.

[sharwell]

:bulb: Whoever implements this can pull the entire implementation and tests aside from the option storage location itself from StyleCop Analyzers rule SA1028.

[jnm2]

I would hope that they would completely ignore any line that isn't parseable.

As in, the editor should preserve the state of the setting prior to parsing the .editorconfig.

[olegtk]

Looks like the issue was previously raised on editorconfig issue tracker: https://github.com/editorconfig/editorconfig/issues/304. We can try again.

I like "trim_trailing_whitespace = safe" suggestion though. As for the value parsing concern - editorconfig recently added a universal "unset" value that can be assigned to any property, which would apparently break every impl using != "false" kind of parsing anyway.

[Pilchie]

I like the idea of safe. @olegtk / @markw-t do you want to shepherd that with the editorconfig community, or do you want @kuhlenh and I to?

[markw-t]

Since this is a core editorconfig feature we should probably kick off the debate with the community. @Pilchie. @olegtk do you want to prep a description for us to post to the editorconfig forum?