search

haskell-servant / servant

Main repository for the servant libraries — DSL for describing, serving, querying, mocking, documenting web applications and more!
https://docs.servant.dev/
1.8k stars 407 forks source link

Add `MkHandler` pattern synonym #1733

Closed danidiaz closed 2 months ago

danidiaz commented 2 months ago

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.

ysangkok commented 2 months ago

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 commented 2 months ago

Yes please produce a changelog.d entry! :)

tchoutri commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.

tchoutri commented 2 months ago

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 :)