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

Add MkHandler pattern synonym #1733

Conversation

danidiaz
Copy link

This bidirectional pattern synonym makes it easier to build Handler a values from IO (Either ServerError a) values, and is a complement to the already existing runHandler function.

image

As an alternative to this, I also considered re-exporting the ExceptT constructor, but I think it's better to have an explicit pattern synonym. ExceptT is not exactly an internal implementation detail, but it's sufficiently close to being one.

As for naming the pattern synonym, I considered HandlerT but it would be a bit misleading because Handler is not really a monad transformer. The prefix is Mk instead of Make because other parts of the Servant ecosystem (like safe links) have already settled on Mk.

Solves #1732.

This bidirectional pattern synonym makes it easier to build `Handler a`
values from `IO (Either ServerError a)` values, and is a complement to
the already existing `runHandler` function.

Solves haskell-servant#1732.
@ysangkok
Copy link
Contributor

I think this is a breaking change per PVP, not sure if we are still making changelog.d entries or not. Another maintainer might want to weigh in.

@tchoutri
Copy link
Contributor

Yes please produce a changelog.d entry! :)

@tchoutri
Copy link
Contributor

Regarding the PVP breaking change, @ysangkok would I love to hear your rationale, this isn't immediately clear to me why it would be such a change.

@ysangkok
Copy link
Contributor

Oh you're right @tchoutri , it's not a breaking change according to the PVP flow chart to only add constructors/functions/non-orphan instances. And I suppose a pattern is like a constructor.

@tchoutri
Copy link
Contributor

So, in my book, adding a (public) constructor can lead to breakages when consumers do total case matches on the value. Patterns are less prone to provoke breakages when added.

@danidiaz
Copy link
Author

According to the PVP:

Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes or classes were changed [...]

Adding a new constructor to an exported datatype is definitely a breaking change. Exhaustive pattern-matches in client code would suddenly become non-exhaustive.

However, I don't think adding a wholly new pattern synonym is a breaking change. Existing pattern-matches will be unaffected.

I guess it could be a breaking change, if some existing set of pattern synonyms were declared COMPLETE and a new pattern synonym were added to that set. That would be analogous to adding a "real" constructor to a datatype. But this is not the case here.

Copy link
Contributor

@tchoutri tchoutri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write a changelog.d entry. :)

@tchoutri tchoutri self-assigned this Apr 24, 2024
@tchoutri
Copy link
Contributor

I guess it could be a breaking change, if some existing set of pattern synonyms were declared COMPLETE and a new pattern synonym were added to that set. That would be analogous to adding a "real" constructor to a datatype. But this is not the case here.

yep, that's not our case here :)

@danidiaz danidiaz requested a review from tchoutri April 24, 2024 18:51
@tchoutri tchoutri requested a review from ysangkok April 24, 2024 18:53
@ysangkok ysangkok merged commit 62f3c4f into haskell-servant:master Apr 24, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants