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.

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

Fix trailing comment on switch case #1136

Merged
merged 2 commits into from
Apr 5, 2017

Conversation

duailibe
Copy link
Member

@duailibe duailibe commented Apr 5, 2017

This changes how we print new lines between switch cases, thus fixing #945.

Instead of inserting lines after a case, this inserts before.

src/printer.js Outdated
const isFirstCase = path.getNode() === path.getParentNode().cases[0];

if (!isFirstCase && util.isPreviousLineEmpty(options.originalText, path.getValue())) {
parts.splice(0, 0, hardline);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can do parts.unshift(hardline)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

@vjeux vjeux merged commit 8e9fb43 into prettier:master Apr 5, 2017
@vjeux
Copy link
Contributor

vjeux commented Apr 5, 2017

Awesome thanks!

@vjeux
Copy link
Contributor

vjeux commented Apr 7, 2017

FYI, this removes all the whitespace between lines inside of a case.

switch (x) {
  case y:
    call();

    break;
}

now outputs

switch (x) {
  case y:
    call();
    break;
}

vjeux added a commit to vjeux/prettier that referenced this pull request Apr 7, 2017
.prettier#1136 accidentally removed all the empty lines between statements inside of switch cases. I just brough back the logic and made sure to only use it for everything but the last line.
@vjeux vjeux mentioned this pull request Apr 7, 2017
vjeux added a commit that referenced this pull request Apr 10, 2017
.#1136 accidentally removed all the empty lines between statements inside of switch cases. I just brough back the logic and made sure to only use it for everything but the last line.
@janicduplessis
Copy link
Contributor

janicduplessis commented Apr 14, 2017

Another thing I noticed is comments over a switch case will now have a line under them, I think we need a way to disambiguate between a comment on the actual case and one on the last line of a case body.

switch (a) {
  case y:
    call();

  // Hello
  case z:
    call();
}

will format to

switch (a) {
  case y:
    call();
  // Hello

  case z:
    call();
}

But this case should still work and stay formatted like this.

switch (a) {
  case y:
    call(); // Hello

  case z:
    call();
}

@vjeux
Copy link
Contributor

vjeux commented Apr 14, 2017

There's been at least 2 rounds of fixes on-top of this commit. Please make sure you test against master :)

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 20, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants