Skip to content

Deduplicate, reorder displayException impl #36

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

Conversation

9999years
Copy link
Contributor

@9999years 9999years commented Apr 8, 2025

The default displayException = show implementation is often used, leading to duplicated output in the AnnotatedException displayException implementation.

Here we make a couple tweaks to that implementation:

  1. If show and displayException return the same output, only one is used.
  2. If only the displayException output is shown, it's not indented.
  3. If both the displayException and show output is shown, the displayException output is shown first.
  4. There is a blank line before the "Annotations:" section, to match the blank line between the displayException and show output.
  5. There is a blank line before the call stack section.

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR.
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts).

The default `displayException = show` implementation is often used,
leading to duplicated output in the `AnnotatedException`
`displayException` implementation.

Here we make a couple tweaks to that implementation:

1. If `show` and `displayException` return the same output, only one is
   used.
2. If only the `displayException` output is shown, it's not indented.
3. If both the `displayException` and `show` output is shown, the
   `displayException` output is shown first.
4. There is a blank line before the "Annotations:" section, to match the
   blank line between the `displayException` and `show` output.
5. There is a blank line before the call stack section.
callStackMessage =
[ ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra newline before the callstack section to match the newline between the displayException and show sections.

anns ->
"Annotations:\n"
<> unlines (map (\ann -> "\t * " <> show ann) anns)
[ ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra newline before the "Annotations:" section to match the newline between the displayException and show sections.

Copy link
Owner

@parsonsmatt parsonsmatt left a comment

Choose a reason for hiding this comment

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

hell yeah!!!

@9999years 9999years marked this pull request as ready for review April 9, 2025 18:31
@parsonsmatt parsonsmatt merged commit 45a180f into parsonsmatt:master Apr 10, 2025
9 checks passed
9999years added a commit to 9999years/annotated-exception that referenced this pull request Apr 11, 2025
In parsonsmatt#36, I restructured the `AnnotatedException` `displayException`
implementation so that instead of being of the form:

    displayException =
        unlines [ ... ] <> callStackMessage
        where
            callStackMessage = prettyCallStack frames

It was of the form:

    displayException =
        unlines $ [ ... ] <> callStackMessage
        where
            callStackMessage = [prettyCallStack frames]

I didn't realize that `unlines` adds a trailing newline, which is
generally the wrong behavior for `displayException` instances.

This change replaces `unlines` with `concat . intersperse "\n"` in order
to regain the correct behavior.
parsonsmatt pushed a commit that referenced this pull request Apr 11, 2025
* displayException: Remove trailing newline

In #36, I restructured the `AnnotatedException` `displayException`
implementation so that instead of being of the form:

    displayException =
        unlines [ ... ] <> callStackMessage
        where
            callStackMessage = prettyCallStack frames

It was of the form:

    displayException =
        unlines $ [ ... ] <> callStackMessage
        where
            callStackMessage = [prettyCallStack frames]

I didn't realize that `unlines` adds a trailing newline, which is
generally the wrong behavior for `displayException` instances.

This change replaces `unlines` with `concat . intersperse "\n"` in order
to regain the correct behavior.

* 0.3.0.3 -> 0.3.0.4
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.

2 participants