-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
checkstyle RightCurly not correct when there are comments #637
Comments
Prettier JavaScript keeps the comment in this position (example), so I don't think we will change the formatting of this. We could potentially update the Checkstyle configuration that we provide. |
I'm leaning towards simply removing the Checkstyle configuration that this repo provides. We haven't been maintaining it, and IMO there isn't really a good reason for Prettier Java to provide a Checkstyle configuration. If you use Checkstyle and Prettier Java, I would imagine you would want to omit/disable any Checkstyle rules that duplicate responsibilities of Prettier Java. At that point, anything left in the Checkstyle ruleset wouldn't have anything to do with Prettier Java, and would just be personal preference. @xenoterracide As it seems you've been using the provided Checkstyle configuration, I'm curious what your thoughts are on the above? Is there a valid use-case for a Checkstyle configuration that matches Prettier Java's style that I'm not considering? |
There are many reasons to use check style in addition to prettier... However let's focus on purely code formatting rules that prettier could cover. Prettier including prettier Java does not comprehensively cover even code files. There are many rules that could be argued though that prettier does cover like this one. Arguably the big reason is not to have to run node in CI on every build. The only node job I have in CI is to update node dependencies. To validate that developers are using prettier's style you would have to install node and the prettier dependencies and run the check in CI. |
Also, one benefit although imperfect... Is that it could theoretically catch a regression in prettier-java. However I have found that check style cannot validate all of What prettier does. In fact I had one of their developers downright insult The style because it wasn't one Three they had seen before... As it's not that yet common to use in the Java world. I would argue though it's the only one that is sensible for lambda programming... |
Sorry, I'm not sure how I messed up that user mention, I meant to direct that question to you. To clarify my question, I'm not asking why one would want to use Checkstyle and Prettier together. There are a number of code-style-related things that Prettier doesn't touch, and so for things like Javadoc style, class/variable/method naming style, etc. it makes sense to use another tool such as Checkstyle. However, it sounds like you're saying you require your devs to install Node.js and Prettier to format code, but don't want to have to do so on CI to validate the code is formatted? There are quite a lot of code style choices that Prettier makes that I don't believe have equivalent Checkstyle rules, so while Checkstyle may work for some teams using Prettier Java, I think our primary recommendation should be for projects to use I don't know that it makes sense for us to provide and maintain these Checkstyle rules, given that we're developing a Prettier plugin, and Prettier has its own means of validating code style. |
Yes that's what I'm saying. Yes you are correct as far as the fact that prettier does a lot of things that they don't and can't do and vice versa. You could take the rules (ideally as corrected from any bugs I've opened) And open a bug over there and ask if they will maintain them like they do the Google and Sun rules. Maybe they would be willing to host a copy. An additional benefit that I don't personally care much about is that some people have check style validation enabled in their IDE. Obviously they should have prettier valid formatting enabled too but it would highlight The errors if Auto formatting was broken. I've had IntelliJ fail at auto formatting so many times due to lack of ease of use of prettier. It simply does not set up node And prettier correctly by default. It doesn't set up checkstyle by default though either. So in both cases whenever I set up a new project I have to go through and correct all of its nonsense. |
**Prettier-Java 2.5.0 **
Input:
I apologize, this hot mess is so long, but I'm wondering if that's part of what's causing the problem. I added a violation in the code here.
Expected behavior:
...
I'm not sure this is wrong per prettier, but I don't see a way to fix per checkstyle either. Please leave open until one or the other can be resolved.
The text was updated successfully, but these errors were encountered: