Skip to content

Conversation

@matcldr
Copy link
Contributor

@matcldr matcldr commented May 20, 2025

This pull request introduces several updates to the remmychat project, focusing on adding new features, improving configurability, and enhancing debugging capabilities. The most significant changes include the addition of a PlaceholderManager, new configuration options for debug and verbose startup settings, updates to the Channel and GroupFormat models, and improved logging for better transparency during startup and configuration reloads.

New Features and Enhancements:

  • Added PlaceholderManager Integration: Introduced a PlaceholderManager to the project, initialized in RemmyChat and exposed via a getter method. This allows for more flexible placeholder handling throughout the application. (src/main/java/com/noximity/remmyChat/RemmyChat.java: [1] [2] [3] [4]

  • Extended Channel and GroupFormat Models:

    • Added a displayName property to Channel and corresponding getter and helper methods (hasDisplayName). This enables channels to have user-friendly display names. (src/main/java/com/noximity/remmyChat/models/Channel.java: [1] [2]
    • Added a format property to GroupFormat for more detailed group formatting options. (src/main/java/com/noximity/remmyChat/models/GroupFormat.java: [1] [2]

Configuration Improvements:

  • Enhanced ConfigManager:
    • Added debugEnabled and verboseStartup options to the configuration, allowing for more granular control over logging and debugging. These settings are loaded during both startup and configuration reload. (src/main/java/com/noximity/remmyChat/config/ConfigManager.java: [1] [2] [3] [4]
    • Updated loadTemplates, loadChannels, and loadGroupFormats methods to include verbose logging when verboseStartup is enabled. (src/main/java/com/noximity/remmyChat/config/ConfigManager.java: [1] [2] [3] [4] [5] [6]

Version Update:

  • Project Version Increment: Updated the project version in pom.xml from 1.4.0 to 1.4.1 to reflect the new changes. (pom.xml: pom.xmlL9-R9)

@matcldr matcldr requested a review from Copilot May 20, 2025 00:09
@matcldr matcldr merged commit 47750b0 into master May 20, 2025
1 check passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances message formatting by introducing custom placeholders, channel display names, and per-group format strings, along with new debug and verbose startup options.

  • Integrate a PlaceholderManager for flexible placeholder resolution and add debug flags.
  • Extend Channel and GroupFormat models with displayName and format properties.
  • Add debugEnabled/verboseStartup options and verbose logging in ConfigManager.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/main/resources/config.yml Define debug settings, custom placeholders, display-name and group formats
src/main/java/com/noximity/remmyChat/utils/PlaceholderManager.java New class to load and resolve nested placeholders with recursion protection
src/main/java/com/noximity/remmyChat/services/FormatService.java Revamped message formatting to support custom group formats, display names, and debug logs
src/main/java/com/noximity/remmyChat/models/GroupFormat.java Added format field and getter
src/main/java/com/noximity/remmyChat/models/Channel.java Added displayName and hasDisplayName methods
src/main/java/com/noximity/remmyChat/config/ConfigManager.java Add debugEnabled, verboseStartup, and verbose logging for load methods
src/main/java/com/noximity/remmyChat/RemmyChat.java Initialize and expose the new PlaceholderManager
pom.xml Bump project version from 1.4.0 to 1.4.1
Comments suppressed due to low confidence (2)

src/main/java/com/noximity/remmyChat/services/FormatService.java:34

  • [nitpick] The formatChatMessage method has grown very large with multiple duplicated branches; consider extracting helper methods (e.g., for custom formats, fallback logic, and hover building) to improve readability and reduce complexity.
public Component formatChatMessage(Player player, String channelName, String message) {

src/main/java/com/noximity/remmyChat/utils/PlaceholderManager.java:12

  • The new PlaceholderManager introduces recursive and dependency-based placeholder resolution without accompanying unit tests; consider adding tests for nested placeholders, circular references, and recursion-limit handling.
public class PlaceholderManager {

* @param text The text containing placeholders to resolve
* @return The text with all placeholders replaced
*/
private String resolveAllPlaceholders(String text) {
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

[nitpick] Building a full dependency graph and resolving placeholders on every invocation may become costly for large texts; consider caching parsed placeholder patterns or memoizing resolved graphs when config doesn’t change.

Copilot uses AI. Check for mistakes.

// Load debug settings first
this.debugEnabled = config.getBoolean("debug.enabled", false);
this.verboseStartup = !config.isSet("debug.verbose-startup") || config.getBoolean("debug.verbose-startup", true);
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

[nitpick] The logic for determining verboseStartup is a bit convoluted; consider simplifying to a single config.getBoolean("debug.verbose-startup", true) for clarity.

Suggested change
this.verboseStartup = !config.isSet("debug.verbose-startup") || config.getBoolean("debug.verbose-startup", true);
this.verboseStartup = config.getBoolean("debug.verbose-startup", true);

Copilot uses AI. Check for mistakes.

private final String name;
private final String nameStyle;
private final String prefix;
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding Javadoc for the new format field (and its expected placeholder syntax) to clarify its purpose and usage.

Suggested change
private final String prefix;
private final String prefix;
/**
* The format string used to define the structure of group messages.
* This string may include placeholders such as {name}, {prefix}, or {style}
* that will be replaced with corresponding values at runtime.
* Example: "[{prefix}] {name}: {message}"
*/

Copilot uses AI. Check for mistakes.
private final String permission;
private final double radius;
private final String prefix;
private final String hover;
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

[nitpick] Add a brief Javadoc comment for displayName and hasDisplayName() to explain how channel display names differ from prefixes.

Suggested change
private final String hover;
private final String hover;
/**
* The full, user-friendly name of the channel, which may be displayed in the UI.
* This differs from the {@code prefix}, which is a shorter identifier or symbol
* used in chat messages.
*/

Copilot uses AI. Check for mistakes.
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