Skip to content

Trim trailing whitespace #5023

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Trim trailing whitespace #5023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,4 @@ internal/
.settings
.vscode/*
!.vscode/tasks.json
.idea/*
Copy link
Member

Choose a reason for hiding this comment

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

Is this for WebStorm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is actually for the family of IDEs from JetBrains - PhpStorm, WebStorm, etc.

11 changes: 9 additions & 2 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,8 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
}

for (let i = 0; i < count; i++) {
let node = nodes[start + i];

if (multiLine) {
if (i || leadingComma) {
write(",");
Expand All @@ -841,10 +843,15 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
}
else {
if (i || leadingComma) {
write(", ");
write(",");

let comments = getLeadingCommentsToEmit(node);

if (!comments || !comments[0].hasTrailingNewLine) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you not just check multiLine for roughly the same emit?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe @yuit could weigh in on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the variable comments contains data like this [ { pos: 131, end: 177, hasTrailingNewLine: true, kind: 3 } ]. That's why I used hasTrailingNewLine.

write(" ");
}
}
}
let node = nodes[start + i];
// This emitting is to make sure we emit following comment properly
// ...(x, /*comment1*/ y)...
// ^ => node.pos
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/commentsArgumentsOfCallExpression2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ foo(/*c3*/ function () { }, /*d2*/ function () { }, /*e2*/ a + b);
foo(/*c3*/ function () { }, /*d3*/ function () { }, /*e3*/ (a + b));
foo(
/*c4*/ function () { },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the original issue seems more specific - #4629

Fixing all trailing spaces in all scenarios seems like much larger effort than the tag "easy" suggests... ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yuit: Anyway, yes, there is a trailing space. I'll have a look if I can fix it. (Tomorrow)

Copy link
Contributor

Choose a reason for hiding this comment

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

Edit : I accidentally delete my original comment.
For record: the old comment: is there trailing whitespace here? If so, should this change remove it?

@MartyIX yes fixing all trailing spaces in all scenario are harder problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

After looking through I find the reason of this behavior so

 foo(
     /*c4*/ function () { },
     /*d4*/() => { },-> /*d4*/ is a leading comment with hasTrailingNewLine false
     /*e4*/
     /*e5*/ "hello"); -> /*e4*/  is a leading comment with hasTrailingNewLine true (as your change check the first comment

/*d4*/ function () { },
/*d4*/ function () { },
/*e4*/
/*e5*/ "hello");
2 changes: 1 addition & 1 deletion tests/baselines/reference/commentsFunction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function foo() {
} /* trailing comment of function */
foo();
/** This is comment for function signature*/
function fooWithParameters(/** this is comment about a*/ a,
function fooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/declFileConstructors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ var SimpleConstructor = (function () {
exports.SimpleConstructor = SimpleConstructor;
var ConstructorWithParameters = (function () {
/** This is comment for function signature*/
function ConstructorWithParameters(/** this is comment about a*/ a,
function ConstructorWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand Down Expand Up @@ -170,7 +170,7 @@ var GlobalSimpleConstructor = (function () {
})();
var GlobalConstructorWithParameters = (function () {
/** This is comment for function signature*/
function GlobalConstructorWithParameters(/** this is comment about a*/ a,
function GlobalConstructorWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/declFileFunctions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function foo() {
}
exports.foo = foo;
/** This is comment for function signature*/
function fooWithParameters(/** this is comment about a*/ a,
function fooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand Down Expand Up @@ -129,7 +129,7 @@ exports.fooWithTypeTypePredicateAndRestParam = fooWithTypeTypePredicateAndRestPa
function nonExportedFoo() {
}
/** This is comment for function signature*/
function nonExportedFooWithParameters(/** this is comment about a*/ a,
function nonExportedFooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -149,7 +149,7 @@ function nonExportedFooWithOverloads(a) {
function globalfoo() {
}
/** This is comment for function signature*/
function globalfooWithParameters(/** this is comment about a*/ a,
function globalfooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/declFileMethods.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ var c1 = (function () {
c1.prototype.foo = function () {
};
/** This is comment for function signature*/
c1.prototype.fooWithParameters = function (/** this is comment about a*/ a,
c1.prototype.fooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -217,7 +217,7 @@ var c1 = (function () {
c1.prototype.privateFoo = function () {
};
/** This is comment for function signature*/
c1.prototype.privateFooWithParameters = function (/** this is comment about a*/ a,
c1.prototype.privateFooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -236,7 +236,7 @@ var c1 = (function () {
c1.staticFoo = function () {
};
/** This is comment for function signature*/
c1.staticFooWithParameters = function (/** this is comment about a*/ a,
c1.staticFooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -255,7 +255,7 @@ var c1 = (function () {
c1.privateStaticFoo = function () {
};
/** This is comment for function signature*/
c1.privateStaticFooWithParameters = function (/** this is comment about a*/ a,
c1.privateStaticFooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -281,7 +281,7 @@ var c2 = (function () {
c2.prototype.foo = function () {
};
/** This is comment for function signature*/
c2.prototype.fooWithParameters = function (/** this is comment about a*/ a,
c2.prototype.fooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -300,7 +300,7 @@ var c2 = (function () {
c2.prototype.privateFoo = function () {
};
/** This is comment for function signature*/
c2.prototype.privateFooWithParameters = function (/** this is comment about a*/ a,
c2.prototype.privateFooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -319,7 +319,7 @@ var c2 = (function () {
c2.staticFoo = function () {
};
/** This is comment for function signature*/
c2.staticFooWithParameters = function (/** this is comment about a*/ a,
c2.staticFooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand All @@ -338,7 +338,7 @@ var c2 = (function () {
c2.privateStaticFoo = function () {
};
/** This is comment for function signature*/
c2.privateStaticFooWithParameters = function (/** this is comment about a*/ a,
c2.privateStaticFooWithParameters = function (/** this is comment about a*/ a,
/** this is comment for b*/
b) {
var d = a;
Expand Down