Skip to content

Remove trailing whitespace from JSDoc comments #26029

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

Merged
merged 1 commit into from
Jul 28, 2018

Conversation

tschaub
Copy link
Contributor

@tschaub tschaub commented Jul 27, 2018

This removes trailing whitespace from JSDoc comments. I'll add additional tests after confirming that this is going in a direction you agree with.

Fixes #25916

@msftclas
Copy link

msftclas commented Jul 27, 2018

CLA assistant check
All CLA requirements met.

@tschaub
Copy link
Contributor Author

tschaub commented Jul 27, 2018

I was initially only running npx jake runtests tests=JsDoc. I see that this is causing other failures when running the complete test suite. I'll dig around more later.

Update - I've modified some additional test cases to reflect the lack of trailing whitespace. I'll keep going after getting confirmation that there is agreement about this change.

@@ -0,0 +1,7 @@
/// <reference path='fourslash.ts'/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test is probably not necessary given the number of other tests updated here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but I thought a test with multiple trailing spaces would make sense.

@ghost ghost requested a review from sandersn July 27, 2018 23:58
//// render() {
//// <div /*1*/ ></div>;
//// <div /*2*/ />
//// }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change was due to mixed line endings in the file. I can remove this part of the change if needed.

@tschaub
Copy link
Contributor Author

tschaub commented Jul 28, 2018

I've updated the remaining tests to reflect the new behavior. Let me know if you think the addition of the quickInfoJsDocTags1.ts test is unnecessary or if you'd like to see additional syntax covered.

@mhegazy mhegazy merged commit 0923771 into microsoft:master Jul 28, 2018
@tschaub tschaub deleted the fix-25916 branch July 28, 2018 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants