Merge rebar aliases into analyzer #139
Conversation
|
|
||
| getAlias :: ErlValue -> Maybe (Text, Text) | ||
| getAlias erl = case erl of | ||
| ErlTuple [ErlAtom (AtomText realname), ErlString _, ErlTuple [ErlAtom (AtomText "pkg"), ErlAtom (AtomText alias)]] -> Just (realname, alias) |
There was a problem hiding this comment.
This pattern match is a beast, but it actually seems like the most idiomatic thing. It's kind of a weird structure to find, functionally, so expressing it as-is seems to make sense.
| ErlTuple [atom "number", ErlInt 5678] -- Literal | ||
| ]] | ||
| ConfigValues | ||
| [ErlTuple [ |
There was a problem hiding this comment.
[nit] Would it make sense to have multiple ErlTuple values in the ConfigValues array for the test now that we made that possible with the ConfigValues type?
There was a problem hiding this comment.
Not sure I understand. It already supports multiple ErlValues.
There was a problem hiding this comment.
Yea that makes sense, I was asking if we should modify the test values to contain multiple ErlValues so that we can test it
There was a problem hiding this comment.
Ah, the real-life test already checks that. It fails if the parser doesn't sit just right. Check here
skilly-lily
force-pushed
the
rebar3-config-merge
branch
from
d6594c4 to
cb43ad1
Compare
|
Merging without resolving nits. We can follow these up later @zlav |
Minor changes required for the config parser, including a newtype for safety.