Skip to content

feat(fmt): Expose ConfigurableFormat #360

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

Merged
merged 8 commits into from
Apr 1, 2025
Merged

feat(fmt): Expose ConfigurableFormat #360

merged 8 commits into from
Apr 1, 2025

Conversation

epage
Copy link
Contributor

@epage epage commented Apr 1, 2025

The goal is to allow people to compose more complex formatting options (e.g. for #349) by exposing the built-in formatting

A breaking change was split out into #361

@epage epage force-pushed the format branch 2 times, most recently from 58bce5d to d545806 Compare April 1, 2025 18:21
@epage epage marked this pull request as draft April 1, 2025 18:22
@epage epage changed the title feat(fmt): Expose ConfigurableFormat feat(fmt)!: Expose ConfigurableFormat Apr 1, 2025
@coveralls
Copy link

coveralls commented Apr 1, 2025

Pull Request Test Coverage Report for Build 14204274818

Details

  • 17 of 50 (34.0%) changed or added relevant lines in 2 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.7%) to 44.113%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/logger.rs 0 11 0.0%
src/fmt/mod.rs 17 39 43.59%
Files with Coverage Reduction New Missed Lines %
src/fmt/mod.rs 3 63.7%
Totals Coverage Status
Change from base Build 14202307123: -0.7%
Covered Lines: 266
Relevant Lines: 603

💛 - Coveralls

@epage epage changed the title feat(fmt)!: Expose ConfigurableFormat feat(fmt): Expose ConfigurableFormat Apr 1, 2025
@epage epage marked this pull request as ready for review April 1, 2025 19:20
@epage epage merged commit 3ca671f into rust-cli:main Apr 1, 2025
15 checks passed
@epage epage deleted the format branch April 1, 2025 19:34
@djugei
Copy link

djugei commented Apr 19, 2025

this seems somewhat redundant, customformat or default format is used, i do not quite understand why both exist at the same time.
tbh i don't fully understand the intent behind this entire pr.

@djugei
Copy link

djugei commented Apr 19, 2025

i do not think the builder which modifies defaultformat would ever be useful for any user of the library that wants a custom formatter. no functions interact at all. i think those two should be fully decoupled.

@epage
Copy link
Contributor Author

epage commented Apr 21, 2025

this seems somewhat redundant, customformat or default format is used, i do not quite understand why both exist at the same time.
tbh i don't fully understand the intent behind this entire pr.

This PR serves the following roles

  • Makes the intent clearer in what is associated with the default formatter and not the custom formatter
  • Allows callers to create their own formatters on top of ConfigurableFormat

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