-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
Formatter off on #289
Formatter off on #289
Conversation
c309f63
to
f744c9f
Compare
good job @nonomoho ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me ! Well done @nonomoho !
Could you remove single-printer-run/_input.java and single-printer-run/_output.java from the commited files ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ! Thanks @nonomoho !
Just two additionals comments:
- it would be great to find crystal clear names for the formatter-on-off functions to distinguish them from the prettier-ignore ones. Maybe separate them in the code and add jsdoc ?
- in the same idea, we could try to call only one method in
packages/java-parser/src/index.js
which contains the logic regarding ignore-comments (prettier-ignore and formatter:(on|off)). Same inpackages/java-parser/src/parser.js
: it would be clearer IMO to just call ashouldIgnore
function in the parser file, which would then call a shouldIgnoreOffOn/PrettierIgnore or something like that
Would it be possible to add some tests at some other places than just before classes definition ? Before methods, binary operations and so on |
aa7ff99
to
393ee63
Compare
Closed with #323 |
fix #287