Skip to content

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