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

Wrap before binary operators #534

Open
JanecekPetr opened this issue Apr 19, 2022 · 12 comments
Open

Wrap before binary operators #534

JanecekPetr opened this issue Apr 19, 2022 · 12 comments

Comments

@JanecekPetr
Copy link

Binary operators are being wrapped in a Java-uncommon way.

Binary operator wrapping/indentation was, as far as I can see, last overhauled in #255 which "fixed" three issues. However, the discussion in #218 was sadly ignored. The ticket predominantly says that long lines should be wrapped before the binary operators, as that's common in Java. And yet the implementation wraps after the operators.

Prettier-Java 1.6.1

# Options (if any):
--print-width 80

Input:

        String path = "/" + someotherlongstuff + "/";

Output:

        String path =
            "/" +
            someotherlongstuff +
            "/";

Expected behavior:

        String path
            = "/"
            + someotherlongstuff
            + "/";

or maybe

        String path = "/"
            + someotherlongstuff
            + "/";

This is accented by the fact that for multi-catch we already wrap by "my" rules:

        } catch (
            IOException
            | FTPException
            | NumberFormatException
            | ParseException e
        ) {

I understand that the decision has been done at some point and is not very likely to change now as there's already lots of formatted code out there. That said, the discussion around the matter was very clear that Java does things differently than the result, and I'm here to stand by that. At least I wanted to revisit the decision and either explicitly close it forever, or leave it open, gather feedback, and maybe change it in a future major version.

@bmslgpn
Copy link

bmslgpn commented Nov 29, 2023

This feature is also asked for the parent project prettier prettier/prettier#3806.
Our team would love having this feature for the java language.

@jtkiesel
Copy link
Contributor

@pascalgrimaud / @clementdessoude What are your thoughts on implementing this change? I believe it should be relatively simple for us to make this change, but I wanted to gauge your opinions before looking into it. The Prettier maintainers themselves seem to be quite against making this change to Prettier itself (at one point it seemed like it might make it into 3.0, but then it was postponed without much explanation.)

IMO this change is basically a no-brainer, as I believe that in most code I've seen (Java in particular), people tend to wrap lines before binary operators, rather than after. Objectively speaking, there is a lot of better evidence and arguments provided by others in the nearly-six-year-old issue thread that @bmslgpn linked to above.

If there are no objections by the maintainers, I'm happy to create a PR making this change.

@pascalgrimaud
Copy link
Member

No really opinion on that :-D
I'll follow the final decision from @clementdessoude

@jhaber
Copy link
Contributor

jhaber commented Dec 1, 2023

I think I chimed in on the original issue about wanting this. But I think formatting is rarely a game of "A is objectively better than B" and more about consistency and what you're used to. At this point our company has millions of lines of code formatted according to the current prettier style, and thousands of engineers used to reading code that way for years. From my perspective I think it's fine to say that the ship has sailed, especially on such a core formatting decision

@bmslgpn
Copy link

bmslgpn commented Dec 4, 2023

@jhaber, I hear you. Though, my company has hundreds of thousands lines of code formatted with operators on the left side and we don't use prettier because of that. Also, I don't see why formatting would not be a "A is objectively better than B" if in the long term, there are objectively more advantages than disadvantages. Your point of view has to be considered nonetheless. My opinion is that a parameter would be great.

@pascalgrimaud
Copy link
Member

I wonder if it's possible to add a key configuration to support both ? So it will make everyone happy, and won't change the default behavior like today

image

@bmslgpn
Copy link

bmslgpn commented Dec 4, 2023

If I can suggest something, it would be awesome to change the behaviour by default and add a "lecagyOperatorPosition" (or something similar) parameter for those wanting to keep the old behaviour.

@jhaber
Copy link
Contributor

jhaber commented Dec 4, 2023

Though, my company has hundreds of thousands lines of code formatted with operators on the left side and we don't use prettier because of that

If people refuse use to prettier because of specific formatting choices, then I would argue that they're not bought into the prettier philosophy and will likely encounter many other formatting quirks that they also disagree with. People get really attached to specific formatting styles, and it is true that reading code in a different style is initially slower and disorienting. But after a week you're used to it and never think about it again (import order is a good example where people think it's so important, but once you stop caring you realize how pointless it was).

prettier-java could add a config option, but prettier is intentionally an opinionated code formatter that is supposed to avoid options (plugins are free to do whatever they want, but I believe prettier core has officially declared they will never add another option). There's no shortage of configurable Java formatters if your goal is to have code formatted according to your existing style.

prettier has a mountain of requests just like prettier/prettier#3806:
https://github.com/prettier/prettier/issues?q=is%3Aissue+is%3Aopen+sort%3Acomments-desc

Changing any of those behaviors will just cause people who want the opposite behavior to open a new issue, and the cycle repeats. However, you could make the argument that:

  • prettier-java doesn't have the same volume of change requests as prettier core
  • in this case, the current behavior diverges from generally accepted Java formatting
  • so adding an option or changing the behavior isn't the same slippery slope that it is for prettier core

It might be late at this point, but it could also be helpful to define a prettier-java philosophy. For example, is the goal to match prettier javascript formatting where possible? What about in cases where that diverges from how Java formatting is normally done? (continuation indent, binary operator linebreaking, generics, etc.)

@bmslgpn
Copy link

bmslgpn commented Dec 4, 2023

If you sort the prettier issues by the most 👍, placing binary operators on the left is the most asked feature
https://github.com/prettier/prettier/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc
Also, it is planned for milestone 4.0.

@jhaber
Copy link
Contributor

jhaber commented Dec 4, 2023

I think 👍 can be misleading, for all we know there's two orders of magnitude more 👎 who never saw the issue because they're too busy writing code and letting prettier handle the formatting

@jtkiesel
Copy link
Contributor

jtkiesel commented Dec 4, 2023

it is true that reading code in a different style is initially slower and disorienting. But after a week you're used to it and never think about it again

@jhaber I wholeheartedly agree, and I think this makes your original concern moot:

At this point our company has millions of lines of code formatted according to the current prettier style, and thousands of engineers used to reading code that way for years.

I think your idea of Prettier Java coming up with its own philosophy would be very helpful for situations such as this. I also agree that based on Prettier's own philosophy, it would likely be a bad idea to make this an option.

If we are open to making formatting changes such as this, but are concerned about which way more users would prefer, and/or what the precedence is in existing code, then we can have that discussion, though I encourage anyone wishing to participate to read through prettier/prettier#3806. I agree that there are issues with only considering the number of thumbs-up an issue such as this has, but I think a lot of the other points/evidence presented by the participants in that thread (related to consistency with "official" language recommendations, other formatters' behavior, and even consistency within Prettier [Java] itself). There is something to be said about whether we want to diverge from Prettier's own style in this regard, though we've already done that when we added empty lines to the start of class bodies (that being said, going back to @jhaber's idea of coming up with a Prettier Java philosophy would be helpful in making that decision).

@jhaber
Copy link
Contributor

jhaber commented Dec 4, 2023

it is true that reading code in a different style is initially slower and disorienting. But after a week you're used to it and never think about it again

@jhaber I wholeheartedly agree, and I think this makes your original concern moot:

At this point our company has millions of lines of code formatted according to the current prettier style, and thousands of engineers used to reading code that way for years.

We already paid a massive one-time cost of reformatting all of our code to get onto prettier. There is a real cost to doing that again to our entire codebase. Both in terms of superfluous diffs polluting our git history, and in the frustration and lost productivity of our engineers who need to get reacquainted with a new code style. I'm trying to weigh this cost against the benefit of optimizing for a particular code style preference.

In terms of next steps it seems like there is:

  • make the change globally
  • add an option
  • wait to see if prettier adopts this behavior

If the prettier-java philosophy is to match prettier core, then the last option might be the most consistent

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

No branches or pull requests

5 participants