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 wrap, fill, refill methods to Options #523

Closed
wants to merge 1 commit into from

Conversation

9999years
Copy link
Contributor

These functions all take an Options as a parameter. I usually construct an Options and then use it for multiple calls, so it's more convenient for me to have access to these as methods.

These functions all take an `Options` as a parameter. I usually
construct an `Options` and then use it for multiple calls, so it's more
convenient for me to have access to these as methods.
@mgeisler
Copy link
Owner

Hi @9999years, thanks for the PR.

This change would make the API similar to what I had before version 0.13 from 2020. So I can fully understand why this API seems natural — I started out with the same one 😄

My feeling back then is summed up a bit in #213, but overall I was concerned about two things:

  • a one-struct-to-rule-them-all in the form of Wrapper (back then) or Options (now). I wanted to avoid an always-expanding struct with more and more methods.
  • having more than one way to call wrap, fill, and friends.

Since 2020, the library has also gotten some simple column wrapping functionality — but you didn't add those rarely-used methods to the Options struct here 😄 I would prefer not having to decide that is important enough to be added as methods to Options.

Does this seem reasonable?

@9999years
Copy link
Contributor Author

Hmm. If we're talking about breaking changes in #521 already, here's my proposal:

  • Options struct with methods for formatting functions (opts.wrap("foo"), opts.fill("foo"), etc.)
  • Top-level formatting functions with default options (textwrap::wrap("foo"), textwrap::fill("foo"), etc.)

@mgeisler
Copy link
Owner

  • Top-level formatting functions with default options (textwrap::wrap("foo"), textwrap::fill("foo"), etc.)

I don't think this will work: we need to know the correct wrapping width at the very least. The library could of course make an "opinionated" guess by using 80 columns or so, but I feel that won't be very popular with most people.

I will not add back the methods that were removed back in 2020 because of the problems I mentioned above. Overall, I hope the API of the crate can:

  • support simple use cases using a very simple API: textwrap::fill("foo bar", 80).
  • support medium-complexity use cases: textwrap::wrap(text, options).
  • support complex use cases by implementing Fragment.

I feel the current API does this pretty well. Having to import both Options and wrap or fill seems okay to me.

@9999years 9999years closed this Oct 30, 2023
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