-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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 Preference-Applied header in response for Prefer: return=representation/headers-only/minimal #2884
Conversation
9e25c56
to
7718c32
Compare
…presentation/minimal/headers-only
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.
LGTM! Great work!
-- | Add headers not already included to allow the user to override them instead of duplicating them | ||
addHeadersIfNotIncluded :: [HTTP.Header] -> [HTTP.Header] -> [HTTP.Header] | ||
addHeadersIfNotIncluded newHeaders initialHeaders = | ||
filter (\(nk, _) -> isNothing $ find (\(ik, _) -> ik == nk) initialHeaders) newHeaders ++ | ||
filter (\(nk, nv) -> isNothing $ find (\(ik, iv) -> ik == nk && nv == iv) initialHeaders) newHeaders ++ | ||
initialHeaders | ||
|
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.
The comment says that it prevents duplication of headers. Seems like this is broken (see #2915). Maybe we should add some doc tests to this function to prove it still does what the comment says...
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.
Ahh! I initially did that to allow multiple Preference-Applied
, however I forgot to change that back later. I'll send a PR.
Fixes issue #740.