Skip to content

Deduplicate, reorder displayException impl#36

Merged
parsonsmatt merged 2 commits intoparsonsmatt:masterfrom
9999years:wiggles/dux-3311-annotated-exception-deduplicate-reorder-show-and
Apr 10, 2025
Merged

Deduplicate, reorder displayException impl#36
parsonsmatt merged 2 commits intoparsonsmatt:masterfrom
9999years:wiggles/dux-3311-annotated-exception-deduplicate-reorder-show-and

Conversation

@9999years
Copy link
Copy Markdown
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.
(callStacks, otherAnnotations) = tryAnnotations @CallStack annotations

callStackMessage =
[ ""
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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