Skip to content
125 changes: 125 additions & 0 deletions src/main/java/dansplugins/mailboxes/Mailboxes.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,48 @@
import dansplugins.mailboxes.services.*;
import dansplugins.mailboxes.utils.*;

import org.bukkit.Bukkit;
import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
import org.bukkit.plugin.java.JavaPlugin;

import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public final class Mailboxes extends JavaPlugin {
private final String pluginVersion = "v" + getDescription().getVersion();

// Config option names for tab completion (in camelCase to match config file)
private static final List<String> CONFIG_OPTIONS = Arrays.asList(
"debugMode",
"maxMessageIDNumber",
"maxMailboxIDNumber",
"preventSendingMessagesToSelf",
"assignmentAlertEnabled",
"unreadMessagesAlertEnabled",
"welcomeMessageEnabled",
"quotesEnabled",
"attachmentsEnabled",
"maxAttachmentStackSize"
);
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The config option 'version' is a valid option in ConfigService but is intentionally excluded from CONFIG_OPTIONS since it cannot be set by users. However, this should be documented with a comment explaining why 'version' is excluded, as it may not be obvious to future maintainers.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment explaining why 'version' is excluded. Commit: e4dbece


// Boolean config options (stored in lowercase for case-insensitive comparison)
private static final Set<String> BOOLEAN_CONFIG_OPTIONS = new HashSet<>(Arrays.asList(
"debugmode",
"preventsendingmessagestoself",
"assignmentalertenabled",
"unreadmessagesalertenabled",
"welcomemessageenabled",
"quotesenabled",
"attachmentsenabled"
));
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The config option constants are duplicated here and should be maintained centrally. The ConfigService already defines these options in its saveMissingConfigDefaultsIfNotPresent method. Consider extracting these to a shared constants class or making ConfigService expose a method that returns the list of valid config options. This will prevent inconsistencies when new config options are added in the future.

Suggested change
// Boolean config options (stored in lowercase for case-insensitive comparison)
private static final Set<String> BOOLEAN_CONFIG_OPTIONS = new HashSet<>(Arrays.asList(
"debugmode",
"preventsendingmessagestoself",
"assignmentalertenabled",
"unreadmessagesalertenabled",
"welcomemessageenabled",
"quotesenabled",
"attachmentsenabled"
));
// Boolean config options (stored in lowercase for case-insensitive comparison).
// Derived from CONFIG_OPTIONS to avoid duplicating constant names.
private static final Set<String> BOOLEAN_CONFIG_OPTIONS = new HashSet<>(
CONFIG_OPTIONS.stream()
.filter(option ->
option.endsWith("Enabled")
|| option.endsWith("Mode")
|| option.startsWith("prevent"))
.map(String::toLowerCase)
.collect(Collectors.toSet())
);

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment noting that BOOLEAN_CONFIG_OPTIONS is derived from CONFIG_OPTIONS to maintain consistency. The constants are kept inline for now as they require manual maintenance when adding new config options (the pattern-based suggestion would incorrectly classify some options). Commit: e4dbece


private final ConfigService configService = new ConfigService(this);
private final Logger logger = new Logger(this);
private final PersistentData persistentData = new PersistentData(logger);
Expand Down Expand Up @@ -82,4 +115,96 @@ public MailboxesAPI getAPI() {
private boolean isVersionMismatched() {
return !getConfig().getString("version").equalsIgnoreCase(getVersion());
}

@Override
public List<String> onTabComplete(CommandSender sender, Command cmd, String label, String[] args) {
if (label.equalsIgnoreCase("mailboxes") || label.equalsIgnoreCase("m")) {
return getTabCompletions(sender, args);
}
return null;
}

private List<String> getTabCompletions(CommandSender sender, String[] args) {
if (args.length == 1) {
// Main subcommands
List<String> subcommands = Arrays.asList("help", "config", "list", "send", "open", "delete", "archive");
return filterCompletions(subcommands, args[0]);
Comment on lines 99 to 109
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

Consider adding permission checks for the main subcommands in tab completion to avoid showing commands that users don't have access to. For example, only show "config" in completions if the user has 'mailboxes.config' permission. This is consistent with how CommandInterpreter.java checks permissions before executing commands and prevents information leakage.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added permission checks for all main subcommands and their completions to prevent information leakage. Commit: e4dbece

}

if (args.length >= 2) {
String subcommand = args[0].toLowerCase();

switch (subcommand) {
case "config":
return getConfigCompletions(args);
case "list":
return getListCompletions(args);
case "send":
return getSendCompletions(sender, args);
}
}

return new ArrayList<>();
}

private List<String> getConfigCompletions(String[] args) {
if (args.length == 2) {
// Config subcommands
return filterCompletions(Arrays.asList("show", "set"), args[1]);
}
if (args.length == 3 && args[1].equalsIgnoreCase("set")) {
// Config options
return filterCompletions(CONFIG_OPTIONS, args[2]);
}
if (args.length == 4 && args[1].equalsIgnoreCase("set")) {
// Suggest values based on the config option type
String option = args[2];
if (BOOLEAN_CONFIG_OPTIONS.contains(option.toLowerCase())) {
return filterCompletions(Arrays.asList("true", "false"), args[3]);
}
}
return new ArrayList<>();
}

private List<String> getListCompletions(String[] args) {
if (args.length == 2) {
return filterCompletions(Arrays.asList("active", "archived", "unread"), args[1]);
}
return new ArrayList<>();
}

private List<String> getSendCompletions(CommandSender sender, String[] args) {
if (args.length == 2) {
// Player names and -attach flag
List<String> completions = filterCompletions(
Bukkit.getOnlinePlayers().stream()
.map(Player::getName)
.collect(Collectors.toList()),
args[1]
);
// Also suggest -attach flag if user has permission
completions.addAll(getAttachFlagSuggestion(sender, args, args[1]));
return completions;
}
// Suggest -attach flag for subsequent positions if not already present
if (args.length >= 3) {
return getAttachFlagSuggestion(sender, args, args[args.length - 1]);
}
return new ArrayList<>();
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The -attach flag suggestion is provided both at args.length == 2 (line 186) and args.length >= 3 (line 191). This means -attach will be suggested at position 2 (which is the player name position) alongside player names. The -attach flag should only be suggested after the player name is provided, not at the same position as the player name. Consider restricting -attach suggestions to args.length >= 3 only.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed -attach flag to only suggest at position 3+ (after player name). Removed from position 2 suggestions. Commit: e4dbece

}

private List<String> getAttachFlagSuggestion(CommandSender sender, String[] args, String input) {
// Check if -attach flag is already present
boolean hasAttachFlag = Arrays.stream(args).anyMatch(arg -> arg.equalsIgnoreCase("-attach"));
if (!hasAttachFlag && sender.hasPermission("mailboxes.send.attach")) {
return filterCompletions(Arrays.asList("-attach"), input);
}
return new ArrayList<>();
}

private List<String> filterCompletions(List<String> options, String input) {
return options.stream()
.filter(option -> option.toLowerCase().startsWith(input.toLowerCase()))
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The filter method uses case-insensitive comparison (.toLowerCase()) but creates new strings for each comparison during streaming. For small lists this is fine, but consider caching the lowercased input outside the stream to avoid repeated toLowerCase() calls on the input string.

Suggested change
return options.stream()
.filter(option -> option.toLowerCase().startsWith(input.toLowerCase()))
String lowerInput = input.toLowerCase();
return options.stream()
.filter(option -> option.toLowerCase().startsWith(lowerInput))

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cached lowercased input outside the stream to avoid repeated toLowerCase() calls. Commit: e4dbece

.collect(Collectors.toList());
}
}