Refactored erl_prettypr.erl #4570
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…es; Remove cond expr
matyasmarkovics
changed the title
Contributor
|
@elbrujohalcon, this might interest you. |
maxnordlund
reviewed
Sep 1, 2021
| fun erl_syntax:user_type_application_arguments/1); | ||
| attribute -> | ||
| Name = erl_syntax:attribute_name(Node), | ||
| case catch erl_syntax:concrete(Name) of |
Contributor
There was a problem hiding this comment.
I think this can be simplified a bit to both avoid an unnecessary catch and make it clear what kind of node are allowed as the attribute name, i.e. an atom node.
Suggested change
| case catch erl_syntax:concrete(Name) of | |
| case erl_syntax:atom_value(Name) of |
maxnordlund
reviewed
Sep 1, 2021
| spec -> | ||
| [SpecTuple] = erl_syntax:attribute_arguments(Node), | ||
| [FuncName, FuncTypes] = erl_syntax:tuple_elements(SpecTuple), | ||
| Clauses = erl_syntax:concrete(FuncTypes), |
Contributor
There was a problem hiding this comment.
I like the trick with concrete/abstract here. Very nice 😄
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was bold enough to start a pet-project refactoring the
erl_prettypr.erlmodule. This has been a long-time coming and I still consider it Work-In-Progress. Never-the-less, I thought I'd share the current state to see if there's interest in this work.I plan to:
Further-more:
hook, these functions would be specific for each Tree-type.I'm aware that there have been attempts to "fix-up" this module and those attempts ended-up in separate stand-alone projects. Despite of that I think there still is a value in improving the code-formatter shipped with Erlang/OTP, as it may be used by other internal libraries.
I've decided to simplify the module before attempting to expand on the Layout-Configurability. So, I consider this PR as stage-1 to be honest towards the end-goal: "Make the layout configurable." - Solving the leading-comma formatting would be a good driving principle for configurabilty.
There are a lot of similarities between Erlang statements, like case-clauses and function-clauses, or between data-structure like lists, tuples and binaries, so I've tried to collapse these cases into one, so that they have the same layout.
The produced code-layout relies only on what's possible with the
prettypr. Only the 4 different layouts:sep/1,par/1,above/2andbeside/2are used. These are surprisingly versatile and I've found thefloatlayout nondeterministic (don't really understand how it supposed to work).With this PR a simple, sensible layout is produced for those who just want it to work without any configuration.