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

Make textwrap an optional dependency #1055

Closed
H2CO3 opened this issue Sep 27, 2017 · 8 comments
Closed

Make textwrap an optional dependency #1055

H2CO3 opened this issue Sep 27, 2017 · 8 comments
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations E-medium Call for participation: Experience needed to fix: Medium / intermediate

Comments

@H2CO3
Copy link

H2CO3 commented Sep 27, 2017

It would be nice to make the textwrap crate an optional dependency, so that those who don't care that much about the nice formatting wouldn't need to install the transitive closure of the dependencies of textwrap (most notably, libc and winapi).

This would also cut down on the quantity of unsafe used (again, transitively) by the dependent crates, should they choose to opt out of pretty help text wrapping.

@kbknapp
Copy link
Member

kbknapp commented Oct 4, 2017

I'd be fine with this, if someone wants an easy PR.

👍

@kbknapp kbknapp added A-help Area: documentation, including docs.rs, readme, examples, etc... D: easy E-medium Call for participation: Experience needed to fix: Medium / intermediate C-enhancement Category: Raise on the bar on expectations labels Oct 4, 2017
@H2CO3
Copy link
Author

H2CO3 commented Oct 4, 2017

Sure thing, I'll make the PR for this one as well.

@H2CO3
Copy link
Author

H2CO3 commented Oct 5, 2017

Looks like we'll need to discuss this one a bit more. Here's the complete problem scenario:

  • The doc says that with featurewrap_help enabled, it will wrap the text respecting the actual with of the terminal rather than assuming 120 columns. It also says that this feature will build the term_size crate as a dependency.
  • This would make me assume that term_size is not necessary when this feature is disabled. However, textwrap is a non-optional dependency, and it has term_size as a non-optional dependency itself. This means that term_size is always a dependency.
  • Does this mean that textwrap should also make term_size an optional dependency; better yet, a non-dependency? As far as I can tell, that crate does not internally rely on knowing the terminal width; it merely provides a convenience callthrough/adaptor interface for term_size. I think that it is poor code organization to hard depend on a crate that is not used directly, only for the sake of providing a convenience adaptor for it.
  • Consequently, I think I'll have to go to the textwrap crate's author and ask him/her to remove term_size as a dependency before I can continue implementing this PR.

@H2CO3
Copy link
Author

H2CO3 commented Oct 5, 2017

The issue referred above has been resolved lightning fast. 👍 Once the next release of textwrap is out, I'll write up the necessary changes as a pull request.

@mgeisler
Copy link
Contributor

mgeisler commented Oct 5, 2017

Textwrap 0.9.0 has been released with this change, have fun! :-)

@kbknapp
Copy link
Member

kbknapp commented Oct 7, 2017

Thanks @mgeisler for being flexible and helping with this!

@H2CO3
Copy link
Author

H2CO3 commented Oct 7, 2017

@kbknapp @mgeisler Thanks for your responsiveness and collaboration!

@mgeisler
Copy link
Contributor

mgeisler commented Oct 8, 2017

Happy to help! I want my little crate to be useful in as many scenarios as possible.

@kbknapp kbknapp mentioned this issue Nov 13, 2017
87 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

3 participants