-
Notifications
You must be signed in to change notification settings - Fork 55
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
Macro parens #190
Comments
We do not remove parens, as this sometimes causes bugs. It seems it does get formatted though. -define(REPORT_INTERVAL, ( timer:seconds(5))). and it formatted to -define(REPORT_INTERVAL, (timer:seconds(5))). Is it okay if we close this issue or did I miss what you were trying to say? |
Yes, erlfmt does not touch parentheses precisely because interactions with macros can be tricky. In particular consider the following: -define(COMPUTE(X, Y), (X) * (Y)).
-define(COMPUTE2(X, Y), X * Y).
?COMPUTE(1 + 2, 3 + 4).
% expands to
(1 + 2) * (3 + 4) == 21.
?COMPUTE2(1 + 2, 3 + 4).
% expands to
1 + 2 * 3 + 4 == 11. The outer parentheses can also change the meaning: -define(COND(X, Y), (X orelse Y)).
-define(COND2(X, Y), X orelse Y).
false andalso ?COND(false, true).
% expands to
false andalso (false orelse true) == false.
false andalso ?COND2(false, true).
% expands to
false andalso false orelse true == true. We still handle whitespace, indentation, etc, but never add or remove |
Right you are; nice one. Thanks. |
I see @michalmuskala already mentioned it here. Since my use case was very simple (as you can see from the example I posted) I didn't think of other consequences. I guess |
doesn't get formatted.
Should you consider removing the parens as so:
?
The text was updated successfully, but these errors were encountered: