codecadwallader / codemaid

CodeMaid is an open source Visual Studio extension to cleanup and simplify our C#, C++, F#, VB, PHP, PowerShell, JSON, XAML, XML, ASP, HTML, CSS, LESS, SCSS, JavaScript and TypeScript coding.
http://www.codemaid.net
GNU Lesser General Public License v3.0
1.92k stars 361 forks source link

code comments that don't have a space after triple-slash don't get cleaned up. #534

Open MarkPflug opened 6 years ago

MarkPflug commented 6 years ago

Environment

Description

Code comments that start with "///<" don't get cleaned up by code maid. The important thing here is that there is no space between the triple-slash and the opening angle bracket.

Steps to recreate

  1. create a new C# file
  2. add a code comment such as "///this is a really long line that should wrap when code maid applies the code formatting rules that force comments to wrap at 80 chararacters, whichs looks really nice and consistent.
  3. Run the code maid comment formatter
  4. Notice that the comment is not modified.

Current behavior

No formatting is performed.

Expected behavior

Comment has proper format cleanup applied.

There might be a good reason that this pattern isn't touched that I'm not aware of. My workaround is to simply do a global search-replace before I clean the document. I've inherited a lot of messy code I'm trying to tidy up.

codecadwallader commented 6 years ago

Thanks for asking the question. Yes this is deliberate behavior that comments must have a space after the slashes in order to be formatted. The reason is that we don't want to clean up commented out code, and usually the presence of a space is the best way to distinguish between:

//int x = 0;

and

// ints make me feel > 0

There are also conventions in StyleCop where four slashes are recommended for commented out code but that's certainly not always in use.

Hope it helps. It sounds like in your scenario a global search-replace is probably the best bet.

MarkPflug commented 6 years ago

I totally agree with you in the case of a normal comment, your reasoning makes perfect sense.

However, in the case of a doc comment with triple slashes, I still think that it would make sense to apply the formatting when there is no space. It seems very unlikely that someone would have a commented out a line of code that started with a "/", which resulted in a triple slash when commented out. It would have to be a division expression that spanned lines.

//var six = 12
///2;

A triple-slash in such a position where it was a commented out code should be producing a CS1587 warning. This would only be true if they have the xml output enabled.

The caveat, of course, is that if the triple-slash is followed immediately by another slash, then normal double slash rules should apply. Indeed, a quad-slash is not interpreted as a doc comment, only a triple-slash followed by a non-slash character.

If my argument isn't convincing enough, so be it; your tool is still extremely useful. Thanks for all your hard work, it has saved me a bunch of time.

codecadwallader commented 6 years ago

Yeah, I understand your perspective. I don't disagree with you so I've switched it over to an enhancement if someone wants to pursue it. Someone else wrote the formatting logic but I think the relevant code would be close to here: https://github.com/codecadwallader/codemaid/blob/master/CodeMaid/Helpers/CodeCommentHelper.cs#L140

w5l commented 6 years ago

It's a good point, really, and easy to fix too. But let's think about the logic a bit:

That means an update to the searching algorithm to only return lines prefixed with // or /// (and VB equivalents).

The question that remains open then, is what to do with triple slash comments without space. Do we forcefully insert a space during formatting, or leave as is? Personally I prefer stricter styling, and would force the space.

MarkPflug commented 6 years ago

My vote would be to force the space. That is the default behavior of the VS formatter: when you type the third slash VS will create the \\\ <summary> block template with spaces. For that reason I think it would be hard to argue against requiring them.

w5l commented 6 years ago

@codecadwallader I agree with @MarkPflug on this. What's your sentiment to changing the comment formatter as described above? It is a breaking change is some cases.

Related to #246 and #314.

MarkPflug commented 6 years ago

I was not suggesting adding spaces after normal comments "//", but only after code-comments "///". The code-comment formatter doesn't touch code comments where the comment doesn't contain that space. Currently a code comment starting with: ///<summary> would not be formatted at all, where... /// <summary> would have the formatting rules applied. That is the issue I was suggesting be changed. The rationale behind not formatting normal comments that don't contain a space //var x = 1; I agree with, since such comments would typically be commented out code.

w5l commented 6 years ago

Understood, we can actually safely assume that any triple slash /// prefixed line is always a comment.

The two related tickets ask the same thing as you do (force insert space), but for double slash // prefixed comments. This obviously touches the "commented out code" detection, and therefore cannot be done automatically. But when a user explicitly runs "format comment under cursor", and the text under the cursor starts with a comment prefix, who are we to say we should not format that text?

Either way, that is beyond the scope of your ticket. I will update code to always consider triple slash prefix a comment, and to force a leading space.