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 support for Base.display_error over ExceptionStack. #756

Merged
merged 2 commits into from
Sep 25, 2021

Conversation

Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Sep 22, 2021

#746 added support for current_exceptions/ExceptionStack (née catch_stack), and augmented a few functions with methods that commonly handle ExceptionStacks. This pull request extends that to support to Base.display_error, which also commonly handles ExceptionStacks. Best! :)

@codecov
Copy link

codecov bot commented Sep 22, 2021

Codecov Report

Merging #756 (b09d8a5) into master (15191cb) will increase coverage by 0.20%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #756      +/-   ##
==========================================
+ Coverage   78.35%   78.56%   +0.20%     
==========================================
  Files           4        4              
  Lines         633      639       +6     
==========================================
+ Hits          496      502       +6     
  Misses        137      137              
Impacted Files Coverage Δ
src/Compat.jl 78.53% <100.00%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15191cb...b09d8a5. Read the comment docs.

@Sacha0
Copy link
Member Author

Sacha0 commented Sep 22, 2021

It looks like Julia 1.1 lacks Base.scrub_repl_backtrace; I will guard the new definition and associated test to limit those to >= Julia 1.2.

@Sacha0 Sacha0 requested a review from NHDaly September 22, 2021 21:07
@Sacha0
Copy link
Member Author

Sacha0 commented Sep 22, 2021

CI appears happy now; review much appreciated :).

@Sacha0
Copy link
Member Author

Sacha0 commented Sep 23, 2021

Rebased over #758. What say you, CI? :)

@Sacha0 Sacha0 force-pushed the sv-display-error branch 2 times, most recently from 7a0e166 to 1e788d1 Compare September 23, 2021 22:32
@Sacha0 Sacha0 requested a review from c42f September 24, 2021 15:28
@Sacha0
Copy link
Member Author

Sacha0 commented Sep 24, 2021

@c42f, might I trouble you to have a look at this related patch as well? Thanks again! :)

Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

Looks good to me, I think this can be merged as is, or for a slight improvement also tweaked to work on 1.0 and 1.1

src/Compat.jl Outdated Show resolved Hide resolved
@Sacha0 Sacha0 merged commit fe75815 into master Sep 25, 2021
@Sacha0 Sacha0 deleted the sv-display-error branch September 25, 2021 01:59
Copy link
Member

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

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

LGTM. This is cool - i didn't realize Compat.jl monkey-patched (type pirated?) methods like this to backport them outside julia itself. Neat! :)

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