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

Do not force a blank line above method #284

Open
lppedd opened this issue Oct 15, 2019 · 10 comments
Open

Do not force a blank line above method #284

lppedd opened this issue Oct 15, 2019 · 10 comments

Comments

@lppedd
Copy link

lppedd commented Oct 15, 2019

Print width: 100

Input code:

new TransactionTemplate().execute(new TransactionCallbackWithoutResult() {
      @Override
      protected void doInTransactionWithoutResult(final TransactionStatus status) {
        backupRepository.save(backup);
      }
    }
);

Expected output:

new TransactionTemplate().execute(
    new TransactionCallbackWithoutResult() {
      @Override
      protected void doInTransactionWithoutResult(final TransactionStatus status) {
        backupRepository.save(backup);
      }
    }
  );

Actual:

new TransactionTemplate()
  .execute(
    new TransactionCallbackWithoutResult() {

      @Override
      protected void doInTransactionWithoutResult(final TransactionStatus status) {
        backupRepository.save(backup);
      }
    }
  );
@lppedd lppedd changed the title Do not force a blank line in anonymous class Do not force a blank line above method Oct 15, 2019
@lppedd
Copy link
Author

lppedd commented Oct 15, 2019

Since needs-discussion has been added, my opinion is that minimum blank lines should not be a thing for Prettier. I'm ok with Prettier stripping unnecessary blank lines, but I'm not ok with it adding lines.

Also, the TS/JS parsers/builders never enforced this behavior.

@jhaber
Copy link
Contributor

jhaber commented Oct 15, 2019

I'm not ok with it adding lines.

I'm not sure I agree with this. Here is an example where I think prettier-java should insert multiple newlines:

package com.example;
import java.util.List;
public class ExampleClass {
  private final int field = 1;
  public void methodOne() {
    // do nothing
  }
  public void methodTwo() {
    // do nothing
  }
}

I think this should format as:

package com.example;

import java.util.List;

public class ExampleClass {
  private final int field = 1;

  public void methodOne() {
    // do nothing
  }

  public void methodTwo() {
    // do nothing
  }
}

In particular I think it should enforce that there's exactly one newline between:

  • package and imports
  • imports and class declaration
  • field declaration and method declaration
  • separate method declarations

There are some more controversial questions like whether there should be a newline between class declaration and field declaration, or between static fields and non-static fields, but I'm hoping that the above list isn't particularly controversial

@lppedd
Copy link
Author

lppedd commented Oct 15, 2019

@jhaber sure, every developer/team has its own habits.
I can agree with all those points, but I'd make an edit

  • separate method declarations, excluding if the method is the first element in class body

I actually edited my fork to solve the issue, and I think this is easily doable.
I don't like wasting lines for nothing honestly.

And remember

Also, the TS/JS parsers/builders never enforced this behavior.

@jhaber
Copy link
Contributor

jhaber commented Oct 15, 2019

  • separate method declarations, excluding if the method is the first element in class body

Yep, sorry if my wording wasn't clear. I was trying to express that there should be a newline between methodOne and methodTwo, not necessarily between the class declaration and methodOne. Maybe "consecutive method declarations" would have been more clear.

Forcing a newline between class declaration and the first method declaration happens to be my preference (especially if there is an annotation present on the method), but I understand this is probably going to be divisive

@murdos
Copy link

murdos commented Oct 15, 2019

Hi. I think the problem you're talking about is #271, isn't it?

@lppedd
Copy link
Author

lppedd commented Oct 15, 2019

@murdos pretty much yes, but it's the opposite.

Regarding the question

Do you think we should always add a blank line at the beginning of class bodies and interfaces bodies ?

I'd vote for no, it's not needed always. In some cases it's reasonable for readability, in others not much.

@jhaber
Copy link
Contributor

jhaber commented Dec 12, 2019

I left my thoughts on #271, maybe we should close this issue and consolidate discussion there?

@noahbetzen-wk
Copy link

I feel like this could easily be added as a configuration option.

@jhaber
Copy link
Contributor

jhaber commented Mar 26, 2020

It could be, but the goal of prettier is to (as much as possible) provide a single, opinionated style rather than supporting every possible formatting preference
https://prettier.io/docs/en/option-philosophy.html

@noahbetzen-wk
Copy link

That's fair. Google's Java formatter has a similar philosophy, and my team at work switched away from it because Google's doesn't even allow customizing line lengths or tab widths.

I only suggested a config because people in the two issues seem to disagree on a decision.

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

No branches or pull requests

5 participants