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 report formatter concept for customization of report output #10

Closed
wants to merge 8 commits into from

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Nov 30, 2023

Adds a ReportFormatter trait and a Reporter::report_with_formatter method allowing customization of formatting the report without reimplementing reporter internals. Currently, we use format_external and format_terms but we can refine formatting in the future as needed e.g. all the explain methods in the reporter.

Adds a DefaultStringReportFormatter which implements the existing format for the DefaultStringReporter.

You can see how much easier this is to use in the example diff at fd30571

zanieb added a commit to astral-sh/uv that referenced this pull request Nov 30, 2023
@zanieb
Copy link
Member Author

zanieb commented Nov 30, 2023

I pass the &impl ReportFormatter type around instead of adding a &dyn ReportFormatter field to the reporter to avoid proliferation of lifetimes. Perhaps a more elegant solution around for someone who wants to fight with the compiler.

@zanieb zanieb marked this pull request as ready for review November 30, 2023 17:23
@Eh2406
Copy link

Eh2406 commented Nov 30, 2023

TLDR. I love it!

I had been working toward something like this in https://github.com/pubgrub-rs/pubgrub/tree/reporter_trait, if there are any of those changes that look like good ideas please take them. But your re-factor seems much more user focused and a better place to start.

I would eventually like our reporting infrastructure to support directly outputting to &mut Write instead of having to be buffered in strings, but having seen your branch it is a re-factor better done separately.

@zanieb
Copy link
Member Author

zanieb commented Nov 30, 2023

Good to hear! Here's an upstream pull request pubgrub-rs#158

I had been working toward something like this in https://github.com/pubgrub-rs/pubgrub/tree/reporter_trait, if there are any of those changes that look like good ideas please take them. But your re-factor seems much more user focused and a better place to start.

Ah interesting. I agree it seems a bit more complicated than we need to get started. I think there are some ideas there we can build on though.

I would eventually like our reporting infrastructure to support directly outputting to &mut Write instead of having to be buffered in strings, but having seen your branch it is a re-factor better done separately.

Agreed!

@zanieb
Copy link
Member Author

zanieb commented Nov 30, 2023

Closing in favor of pubgrub-rs#158

@zanieb zanieb closed this Nov 30, 2023
zanieb added a commit to astral-sh/uv that referenced this pull request Dec 1, 2023
Uses astral-sh/pubgrub#10 to drastically simplify
our reporter implementation. This will allow us to make use of upstream
improvements to the reporter e.g.
astral-sh/pubgrub#8 without multiple duplicative
pull requests.
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