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.

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

Jump to bottom

Fix: multiline-comment-style rule add extra space after * (fixes #12785) #12823

Merged
merged 5 commits into from Jan 27, 2020
Merged

Fix: multiline-comment-style rule add extra space after * (fixes #12785) #12823

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2f6afb7
Fix: multiline-comment-style rule missed an extra space after * (fix…
karthikkp Jan 22, 2020
fc2ba59
logic changed to handled only the edge case
karthikkp Jan 23, 2020
1ef1354
test case added for the edge case
karthikkp Jan 23, 2020
add0e21
checks full offset and apply space after * only if there already isn'…
karthikkp Jan 26, 2020
73c99ec
addeed new test case
karthikkp Jan 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/rules/multiline-comment-style.js
View file
Open in desktop
Expand Up @@ -315,7 +315,11 @@ module.exports = {
const lineTextToAlignWith = sourceCode.lines[firstComment.loc.start.line - 1 + idx];
const [, prefix = "", initialOffset = ""] = lineTextToAlignWith.match(/^(\s*(?:\/?\*)?(\s*))/u) || [];

offset = `${commentTextPrefix.slice(prefix.length)}${initialOffset}`;
if (/^\s*\//u.test(lineText)) {
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
offset = `${commentTextPrefix.slice(prefix.length)}${initialOffset} `;
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
} else {
offset = `${commentTextPrefix.slice(prefix.length)}${initialOffset}`;
}
break;
}

Expand Down
19 changes: 19 additions & 0 deletions tests/lib/rules/multiline-comment-style.js
View file
Open in desktop
Expand Up @@ -1379,6 +1379,25 @@ ${" "}
{ messageId: "missingStar", line: 4 },
{ messageId: "endNewline", line: 4 }
]
},
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this test suite a bit more robust by adding other kinds of formatted comments being formatted to starred-block?

Some examples:

////This comment is in
//`separate-lines` format.

// // This comment is in
// `separate-lines` format.


/*
  * // a line comment
 *some.code();
 */


/*
// a line comment
 * some.code();
 */

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 first two are not converted to starred block comments in the current version. So the fix that I am working should not change that behavior right?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, interesting. Yes, that makes sense to me. Thanks for adding these tests.

{
code: `
/*
// a line comment
some.code();
*/
`,
output: `
/*
* // a line comment
*some.code();
*/
`,
options: ["starred-block"],
errors: [
{ messageId: "missingStar", line: 3 },
{ messageId: "missingStar", line: 4 }
]
}
]
});