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

Parameterized macros in function head cause bad formatting #295

Open
onno-vos-dev opened this issue Feb 5, 2022 · 7 comments
Open

Parameterized macros in function head cause bad formatting #295

onno-vos-dev opened this issue Feb 5, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@onno-vos-dev
Copy link

onno-vos-dev commented Feb 5, 2022

Describe the bug
There seems to be some odd bug when it comes to formatting parameterized macros in a function head. Albeit it a very special use-case it is valid syntax and causes the formatter to produce totally wonky code :-)

To Reproduce

-module(test).

-export([test/1]).

-define(macro1(Type), Type).
-define(macro2, macro2).

test(?macro1(?macro2)) ->
     {foo, ?macro2};
test(?macro2) ->
  ?macro2.

Results in:

test( ?macro1( ?macro2 ) ) -> { foo , ?macro2 } ; test( ?macro2 ) -> ?macro2 .

Expected behavior

test(?macro1(?macro2)) ->
  {foo, ?macro2};
test(?macro2) ->
  ?macro2.

Additional context
Options used:

Opts =
  #{formatter => default_formatter,
    paper => 100,
    parse_macro_definitions => true, %% Having this value as true | false makes no difference either way
    truncate_strings => true,
    parenthesize_infix_operations => true,
    break_indent => 2,
    output_dir => current},
@elbrujohalcon
Copy link
Collaborator

Yeah… macros in weird places…
This is somewhat related to #211 and other such tickets…

As per tradition… I'm not saying that you should not use macros that way… but… I wonder what's the actual use case for such code and why can't the same problem be solved without macros. 🤔

@elbrujohalcon elbrujohalcon added this to the Eventually 🙄 milestone Feb 7, 2022
@onno-vos-dev
Copy link
Author

onno-vos-dev commented Feb 7, 2022

Surely it can be done without macros but in our case, there's some short-hand macros for "complicated" tuples which are used in function-heads to determine the code flow. So rather than constantly writing out weird tuples, we use a macro which is short and concise and describes what the macro actually compiles down to better than just seeing this "weird tuple".

So yes, can it be done differently? Sure. Is it sometimes cleaner to do it this way? I'd say so.

Appreciate the fast feedback! <3

@elbrujohalcon
Copy link
Collaborator

I see. If you want to try something… it's possible that using the same macros in case statements instead of function heads… works fine.

test(Thing) ->
  case Thing of
    ?macro1(?macro2) -> {foo, ?macro2};
    ?macro2 -> ?macro2
  end.

@onno-vos-dev
Copy link
Author

Yes that does work however it's not going to be particularly pretty in my case 😢 (Sorry cannot disclose the actual function but it won't be pretty with a long listing of case-clauses (neither is it particularly pretty now but it looks better visually than a long case-clause-listing at least IMHO.

Any idea where I'd go about poking to get this to work? I'm happy to take a look but no promises :-)

@elbrujohalcon
Copy link
Collaborator

I know where to look, but you won't like the answer…
You have to fix erl_syntax, erl_parse, and/or ktn_dodger… to properly understand macros instead of working around them :/

@elbrujohalcon
Copy link
Collaborator

I have a better solution for you… if you feel like implementing something, maybe you can work on #244 … so that you can surround your conflictive macro sections with "no-format" blocks.
That would probably be easier to do, in all likelyhood.

@elbrujohalcon
Copy link
Collaborator

For #244… You might want to start with erlang/otp#5689, actually :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants