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 term_size an optional dependency #101

Closed
H2CO3 opened this issue Oct 5, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@H2CO3
Copy link

commented Oct 5, 2017

Currently, term_size is a required dependency of this crate. This means that all consumers of this library must also install the transitive closure of term_size's dependencies, even those who are not willing to use its functionality.

This is an especially important consideration given that term_size makes heavy use of unsafe, so crates with a zero-unsafe policy (such as one I am currently authoring) may not be able to use textwrap even though they don't actually need any of its unsafe-dependent functionality.

Since with_termwidth() is more or less trivial, and the termwidth() function literally just wraps term_size with a hardcoded default value, it seems to me that it is not the right trade-off that has been made in this regard.

Consequently, I suggest one of the following, in decreasing order of preference:

  • Remove the term_size dependency, termwidth(), and with_termwidth() altogether, because the latter two are trivial and the default value of 80 might not be suitable for everyone anyway.
  • Make with_termwidth() and termwidth() a non-default, optional feature and term_size an optional dependency, so they are not included by default, but those who are willing to use the functionality can still continue doing so without breaking code changes.
  • Make with_termwidth() and termwidth() a default, optional feature and term_size an optional dependency, so they are included by default, but those who are not willing to use the functionality can opt out.

This is blocking clap-rs/clap#1055.

mgeisler added a commit that referenced this issue Oct 5, 2017

cargo: make term_size optional
The term_size crate can be used to determine the current terminal
width and we included a trivial convenience wrapper around it.

However, term_size makes heavy use of unsafe Rust and this was a
concern to some users of textwrap. This commit makes the dependency a
non-default optional feature.

Fixes #101.

mgeisler added a commit that referenced this issue Oct 5, 2017

cargo: make term_size optional
The term_size crate can be used to determine the current terminal
width and we included a trivial convenience wrapper around it.

However, term_size makes heavy use of unsafe Rust and this was a
concern to some users of textwrap. This commit makes the dependency a
non-default optional feature.

Fixes #101.
@H2CO3

This comment has been minimized.

Copy link
Author

commented Oct 5, 2017

@mgeisler 👍

@mgeisler mgeisler closed this in #102 Oct 5, 2017

@mgeisler

This comment has been minimized.

Copy link
Owner

commented Oct 5, 2017

@H2CO3 Thanks for the idea — you're right that it's just a matter of a trivial wrapper and that we can leave it out by default.

@H2CO3

This comment has been minimized.

Copy link
Author

commented Oct 5, 2017

@mgeisler Great, I'm glad you found this useful. 🙂

@mgeisler mgeisler changed the title Remove `term_size` as a (hard required) dependency Make term_size an optional dependency Apr 28, 2018

mgeisler added a commit that referenced this issue Apr 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.