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 default text/plain error formatting #1306

Closed
wants to merge 1 commit into from

Conversation

jFransham
Copy link

Closes #622

I refactored defaultErrorHandler to be a case statement so I could put the helper functions in a where block. If that's bad style or unnecessary change I can get rid of it. Also, I couldn't find any tests for error messages returning the correct content-type, so I'd rather ask about that here than try to write my own and end up reinventing the wheel (obviously if there really are no tests for that, I'll be happy to write them).

<h1>Not logged in
<p style="display:none;">Set the authRoute and the user will be redirected there.
|]
defaultErrorHandler resp = case resp of
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to switch back to the original function pattern matching in place of the case statement, and just move the where clause definitions to top level functions? It would make it much easier to see in the history what the actual change here was.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I thought it was possible this would be a problem.

@snoyberg
Copy link
Member

snoyberg commented Feb 5, 2017

Bump

@StevenXL
Copy link
Member

@snoyberg I think it would be appropriate to close this PR given that #1520 was merged.

@snoyberg
Copy link
Member

Good call

@snoyberg snoyberg closed this Jun 14, 2018
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