-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix some bad jsdoc comment indent #30838
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
Conversation
Solves the initial problem but breaks commentCommentParsing. I also found a couple more interesting cases.
I may try do a little more; `margin += tag.end - tag.pos` bothers me a bit.
I added a few reviewers who might have parsed things in the past. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some explanatory comments
@@ -6441,7 +6441,6 @@ namespace ts { | |||
// for malformed examples like `/** @param {string} x @returns {number} the length */` | |||
state = JSDocState.BeginningOfLine; | |||
margin = undefined; | |||
indent++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was just wrong, as far as I can tell, but only was used for tags followed by a newline:
@ex
stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is, all our previous tests cases looked like this:
@param x comment
comment
indent
Never like this:
@example
comment
comment
indent
so this erroneous increment was hidden by the fact that indent
had already passed margin
.
} | ||
|
||
function parseTag(indent: number) { | ||
function parseTag(margin: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
margin is the thing that's fixed, whereas indent goes up for each line. So this name was wrong before.
@@ -6646,7 +6659,7 @@ namespace ts { | |||
const whitespace = scanner.getTokenText(); | |||
// if the whitespace crosses the margin, take only the whitespace that passes the margin | |||
if (margin !== undefined && indent + whitespace.length > margin) { | |||
comments.push(whitespace.slice(margin - indent - 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this math was also wrong before, for the same reason as the indent++
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me 👍🏽
Fixes #15749 plus a few other cases I discovered.
Yes, I know the baseline format is horrible. Sorry.