Skip to content

feature request: Allow attaching Reports to Reports#208

Open
afmireski wants to merge 6 commits intoeyre-rs:masterfrom
afmireski:198-feature-allow-attaching-reports-to-reports
Open

feature request: Allow attaching Reports to Reports#208
afmireski wants to merge 6 commits intoeyre-rs:masterfrom
afmireski:198-feature-allow-attaching-reports-to-reports

Conversation

@afmireski
Copy link
Copy Markdown

Implemented a way to properly link additional color_eyre::Reports to a Report: report and with_report.

  • All existing tests passed
  • Documentation for new methods added
  • Usage examples provided

closes #198

* Define help_info_report styles for Theme in src/config.rs (Similar to
help_info_error style)

* Implement Display and Debug formatters for HelpInfo::Report
- Added doc comments for both methods
- Added two examples for demonstrate methods usage in /examples:
multiple_reports_lazy.rs and multiple_reports
@afmireski
Copy link
Copy Markdown
Author

Accepted suggestions from cargo fmt and cargo clippy to fix the CI errors related to rustfmt and clippy.

  • All existing tests passed

@afmireski
Copy link
Copy Markdown
Author

afmireski commented Nov 22, 2024

Regarding the macOS-latest CI failure, I believe updating the pyo3 crate to a more recent version that supports Python 3.13 might resolve the issue.
image

Since version 22.0, pyo3 seems to be adding support for Python 3.13.
However, I consider upgrading package versions a critical decision that shouldn't be made alone.

My proposal is to update the eyre Cargo.toml as follows:

[dependencies]
...
pyo3 = { version = "0.23.1", optional = true, default-features = false }
...

[dev-dependencies]
...
pyo3 = { version = "0.23.1", default-features = false, features = ["auto-initialize"] }

@yaahc, would this be a valid approach? I'm open to any suggestions or feedback.

@yaahc
Copy link
Copy Markdown
Collaborator

yaahc commented Nov 22, 2024

tenatively that sounds fine, the only concern would be whats the MSRV for that version of pyo3 since we'd inherit it

@afmireski
Copy link
Copy Markdown
Author

Looking at the release history of the pyo3 crate, the current version being used (0.20) has a MSRV of 1.56.0. Starting from version 0.22.0, the MSRV was raised to 1.63.0.
Is it okay if I proceed with the change?

- Refactored eyre/tests/test_pyo3.rs to support pyo3 version upgrade.
@afmireski
Copy link
Copy Markdown
Author

Bump pyo3 crate version to 0.23.1.

  • All existing tests passed

Observations:

  • The MSRV for Eyre remains 1.65.0 since pyo3 0.23.1 has an MSRV of 1.63.0.
  • test_pyo3.rs was refactored to fix some compiler errors caused by the pyo3 version upgrade.

@afmireski
Copy link
Copy Markdown
Author

Hi, @yaahc I hope you're doing well! When you have some time, could you kindly take a look at my PR? Let me know if anything needs adjustment. Thanks!

"{}",
error.style(theme.help_info_report)
)?;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you have any backtrace or spantrace information in this report, it won't be printed. That feels like a motivating use case for this feature

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hello @vultix , thank you very much for the feedback!

If it's not too much trouble, could you explain in more detail what seems to be wrong?

When running the examples I added, the backtrace information appears to be displayed correctly, very similar to how it is shown when running the error examples:
image

I would really appreciate any clarification you can provide.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The motivation in #198 (by me) was "render [...] including helpful sections like from wrap_err or with_suggestion".

Your screenshot does show one backtrace. It's for the parent error, which is regular behavior, unaffected by this PR.
But there should be three backtraces shown. The two "Report" sections at the bottom, the actual new feature, only show an error message, when they should show all their HelpInfo sections. That's what would set this apart from the existing with_error.

@Rolv-Apneseth
Copy link
Copy Markdown

This would be a really nice feature to have, but I agree that having the backtrace for all reports is quite important. Does the PR need any kind of help to get there @afmireski?

@LeoniePhiline
Copy link
Copy Markdown
Contributor

The removal of the pyo3 feature in #278 obsoletes some of the above discussion and will slightly simplify this PR.

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.

Feature request: Allow attaching Reports to Reports

6 participants