-
Notifications
You must be signed in to change notification settings - Fork 696
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
[WIP] Implement External Custom Setup #4055
Conversation
- `custom-setup` can contain `setup-tool` or `setup-depends` - parses both ways - check to make sure both `setup-*` fields aren't give n
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @phadej, @ezyang and @dcoutts to be potential reviewers. |
|
||
parse = do name <- parse | ||
_ <- Parse.char ':' | ||
exe <- Parse.munch1 isAlphaNum |
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.
I think the executable name format is more flexible than AlphaNum. Safest is to parse as if it were a PackageName.
parsec = do | ||
name <- lexemeParsec | ||
_ <- P.char ':' | ||
exe <- P.munch1 isAlphaNum |
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.
Don't forget to update here too when you're done.
(x, "") -> x | ||
(x, y) -> error "Ambiguous values for test field: '" | ||
where combine field = field a `mappend` field b | ||
combine' field = case ( unUnqualComponentName $ field a |
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.
I'd much prefer it if you could add a function like "isNullUnqualComponentName" to factor this type of test. It would be more efficient too since unUnqualComponentName requires serializing to String.
@Ericson2314 Should we close this PR for now? |
Sure. It will look completely different rebased, and the point won't be external custom setups anyways. |
Implementation of #4047