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

Treat single-star comments as JSDoc #5206

Merged
merged 9 commits into from Oct 23, 2018

Conversation

Projects
None yet
4 participants
@j-f1
Member

j-f1 commented Oct 8, 2018

Fixes #5194. Are the other changes to the snapshots reasonable?

@j-f1

This comment has been minimized.

Member

j-f1 commented Oct 8, 2018

Alternate heuristic: either a /** ... */ single-line comment, or every line except the first and last starts with a *, the first is either empty or *, and the last is empty.

@j-f1 j-f1 added the status:wip label Oct 8, 2018

@lydell

lydell approved these changes Oct 8, 2018

I know you marked this as WIP, but I like it already :)

@lydell

This comment has been minimized.

Collaborator

lydell commented Oct 13, 2018

@j-f1 How do you feel about this one?

@j-f1

This comment has been minimized.

Member

j-f1 commented Oct 13, 2018

Sorry, I haven’t had a chance to refactor the heuristic yet @lydell. I’ll take a look at that this afternoon/evening (EDT).

j-f1 added some commits Oct 14, 2018

@lydell

This comment has been minimized.

Collaborator

lydell commented Oct 14, 2018

Is this an intended change?

Prettier 1.14.3
Playground link

--parser babylon

Input:

  /** one
   * two
   */

Output:

/** one
 * two
 */

Prettier pr-5206
Playground link

--parser babylon

Input:

  /** one
   * two
   */

Output:

/** one
   * two
   */
@j-f1

This comment has been minimized.

Member

j-f1 commented Oct 14, 2018

Sort of. Should I allow arbitrary text on the first line in both /** and /* comments, or just /* ones?

@lydell

This comment has been minimized.

Collaborator

lydell commented Oct 14, 2018

First line: Anything goes, I’d say. I can’t see a reason to restrict the first line in any way?

@lydell

This comment has been minimized.

Collaborator

lydell commented Oct 14, 2018

Perhaps the function should be called something like isIndentableBlockComment rather than isJSDocComment

@j-f1 j-f1 removed the status:wip label Oct 14, 2018

@lydell

This comment has been minimized.

Collaborator

lydell commented Oct 14, 2018

Could you add these tests?

  /** first line
   * second line
   * third line */

  /* first line
   * second line
   * third line */
@lydell

This comment has been minimized.

Collaborator

lydell commented Oct 14, 2018

Btw, would this work?

function isIndentableBlockComment(comment) {
  // If the comment has multiple lines and every line starts with a star
  // we can fix the indentation of each line. The stars in the `/*` and
  // `*/` delimiters are not included in the comment value, so add them
  // back first.
  const lines = `*${comment.value}*`.split("\n");
  return lines.length > 1 && lines.every(line => line.trim()[0] === "*");
}

EDIT: Updated code.

@lydell

This comment has been minimized.

Collaborator

lydell commented Oct 21, 2018

@j-f1 I took the liberty to update your branch with my suggestions myself. What do you think?

@j-f1

This comment has been minimized.

Member

j-f1 commented Oct 21, 2018

LGTM. Good to merge? (cc @duailibe too)

@lydell lydell merged commit 6a17465 into prettier:master Oct 23, 2018

10 checks passed

ci/circleci: build_prod Your tests passed on CircleCI!
Details
ci/circleci: checkout_code Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node4 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node9 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_standalone Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 80%)
Details
codecov/project 96.38% (+0.18%) compared to 303f344
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018

@j-f1 j-f1 deleted the j-f1:single-star-jsdoc branch Oct 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment