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

[Issue 1708] Ignore invalid values in setUsageHelpLongOptionsMaxWidth #1712

Merged
merged 1 commit into from Jun 19, 2022

Conversation

fabio-franco
Copy link
Contributor

This is a fix proposal for #1708.

The idea is to avoid exception when the screen is too small and just let setUsageHelpLongOptionsMaxWidth set the newValue when valid.

@fabio-franco fabio-franco changed the title Ignore invalid values in setUsageHelpLongOptionsMaxWidth [Issue 1708] Ignore invalid values in setUsageHelpLongOptionsMaxWidth Jun 19, 2022
@remkop remkop merged commit 8041ddf into remkop:main Jun 19, 2022
@remkop
Copy link
Owner

remkop commented Jun 19, 2022

Thank you for the contribution!

Copy link
Owner

@remkop remkop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am okay with not throwing the exception but I do think we to log a message when ignoring the user-specified value. Can you take a look?

} else if (newValue > width() - DEFAULT_USAGE_LONG_OPTIONS_WIDTH) {
throw new InitializationException("Invalid usage long options max width " + newValue + ". Value must not exceed width(" + width() + ") - " + DEFAULT_USAGE_LONG_OPTIONS_WIDTH);
if (newValue >= DEFAULT_USAGE_LONG_OPTIONS_WIDTH && newValue <= width() - DEFAULT_USAGE_LONG_OPTIONS_WIDTH) {
longOptionsMaxWidth = newValue;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with not throwing an exception, but I would like to log an INFO-level trace message that tells the user that we are ignoring the specified value, and why (similar to the message text of the InitializationException).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it totally makes sense. I will update the branch and send you an update soon.

@remkop remkop added this to the 4.7 milestone Jun 27, 2022
@remkop remkop added type: enhancement ✨ type: API 🔌 theme: usagehelp An issue or change related to the usage help message labels Jun 27, 2022
remkop added a commit that referenced this pull request Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: usagehelp An issue or change related to the usage help message type: API 🔌 type: enhancement ✨
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The setUsageHelpLongOptionsMaxWidth method launchs exception when running on terminal with small width
2 participants