-
-
Notifications
You must be signed in to change notification settings - Fork 20
GH-983 Add configurable settings for invalid usage message generation #1163
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a configurable way to display invalid command usage messages, which is a great enhancement for user experience. The implementation is well-structured, with new settings neatly organized into their own package and integrated into the main configuration. The use of an interface for the settings is a good design choice for decoupling. I have one suggestion to make the code in the handler slightly more modern and concise.
int limit = Math.max(1, this.invalidUsageSettings.detailedMaxEntries()); | ||
int shown = 0; | ||
|
||
for (String schema : schematic.all()) { | ||
this.noticeService.viewer(viewer, translation -> translation.argument().usageMessageEntry(), SCHEME.toFormatter(schema)); | ||
if (shown++ >= limit) { | ||
break; | ||
} | ||
this.noticeService.viewer( | ||
viewer, | ||
translation -> translation.argument().usageMessageEntry(), | ||
SCHEME.toFormatter(schema)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loop for sending usage hints can be simplified by using the Java Stream API. This approach is more concise, expressive, and less prone to manual indexing errors.
schematic.all().stream()
.limit(Math.max(1, this.invalidUsageSettings.detailedMaxEntries()))
.forEach(schema -> this.noticeService.viewer(
viewer,
translation -> translation.argument().usageMessageEntry(),
SCHEME.toFormatter(schema)
));
this.noticeService.viewer(viewer, translation -> translation.argument().usageMessageHead()); | ||
} | ||
|
||
int limit = Math.max(1, this.invalidUsageSettings.detailedMaxEntries()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int limit = Math.max(1, this.invalidUsageSettings.detailedMaxEntries()); | |
int limit = Math.max(1, this.invalidUsageSettings.detailedMaxEntries()); | |
schematic.all().stream() | |
.limit(limit) | |
.forEach(schema -> this.noticeService.viewer( | |
viewer, | |
translation -> translation.argument().usageMessageEntry(), | |
SCHEME.toFormatter(schema))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pardon nie widziałem podpowiedzi bota xD
@Comment({ | ||
"# When usageHintMode is DETAILED: maximum number of usages to show in the list." | ||
}) | ||
public int detailedMaxEntries = 5; | ||
|
||
@Comment({ | ||
"# When usageHintMode is DETAILED: whether to show the usage header before the list." | ||
}) | ||
public boolean detailedShowHeader = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Comment({ | |
"# When usageHintMode is DETAILED: maximum number of usages to show in the list." | |
}) | |
public int detailedMaxEntries = 5; | |
@Comment({ | |
"# When usageHintMode is DETAILED: whether to show the usage header before the list." | |
}) | |
public boolean detailedShowHeader = true; | |
@Comment({ | |
"# Applies ONLY when usageHintMode = DETAILED.", | |
"# Maximum number of usages to show in the list." | |
}) | |
public int detailedMaxEntries = 5; | |
@Comment({ | |
"# Applies ONLY when usageHintMode = DETAILED.", | |
"# Whether to show the usage header before the list." | |
}) | |
public boolean detailedShowHeader = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest git imo
@SuppressWarnings({"FieldMayBeFinal", "FieldCanBeLocal"}) | ||
@Getter | ||
@Accessors(fluent = true) | ||
public class InvalidUsageConfig extends OkaeriConfig implements InvalidUsageSettings { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to define standards regarding configs in future meeting - why are we having public fields despite having generated getters?
Using

MOST_RELEVANT
:Using

DETAILED
:Video:
2025-09-16.00-07-59.mp4