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 max_width of help configurable #340

Merged
merged 6 commits into from
Feb 6, 2024
Merged

Conversation

ysndr
Copy link
Contributor

@ysndr ysndr commented Feb 4, 2024

bpaf currently applies column wrapping at a hardcoded width of 100 characters.

This PR aims to make this width configurable via the option parser.
Additionally it contributes a fix to the breaking logic where earlier only the current cursor position was used to determine, resulting in the trailing word overflowing the set width of 100.

@ysndr
Copy link
Contributor Author

ysndr commented Feb 4, 2024

Draft, because I mean to add derive support and docs, but I'd like to gather a response to the general approach, before spending more time on it.

@pacak
Copy link
Owner

pacak commented Feb 4, 2024

I'm not opposed to making it configurable even though I'm curious about the motivation.

Implementation looks about right.

@ysndr
Copy link
Contributor Author

ysndr commented Feb 4, 2024

For once 100 characters might not be universally agreed upon (terminals commonly defaulting to 80, ssh reporting a term width of 80, etc).
Proposing to change bpafs default to another opinionated value seemed hypocritical.
Additionally, providing a config like this allows to adapt to the reported terminal width as well if so desired by the calling application.

@pacak
Copy link
Owner

pacak commented Feb 4, 2024

Let me know if you need any help with that change :)

@ysndr ysndr marked this pull request as ready for review February 5, 2024 22:06
@ysndr ysndr force-pushed the console/max_width branch 2 times, most recently from 60bb98e to f4f67df Compare February 5, 2024 22:12
@ysndr
Copy link
Contributor Author

ysndr commented Feb 5, 2024

Alright, derive support is in as well now, and fixed the tests that broke due to including the next word width in the breaking decision.

@pacak
Copy link
Owner

pacak commented Feb 5, 2024

CI failure is not related to your changes. I guess I need to decide how much effort I'm willing to put into supporting rustc 1.56... Will try to fix it tomorrow-ish.

@pacak pacak force-pushed the console/max_width branch from f4f67df to e974fe6 Compare February 6, 2024 14:47
@pacak pacak merged commit 5ee06be into pacak:master Feb 6, 2024
3 checks passed
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