CommandHelper

CommandHelper

46.5k Downloads

Logging is not consistent

LadyCailinBot opened this issue ยท 1 comments

commented

CMDHELPER-3163 - Reported by PseudoKnight

We need to do a pass over the logging in CH to make it more consistent and work on various platforms correctly. (some platforms show [CommandHelper] twice in some locations, other times not at all) This means we need to use the plugin logger if available, and make it accessible from a singular location like Static. I propose Static.getLogger(). I'm not sure how CHLog would fit into this, though, as that seems to be more for warnings/errors. Maybe it should be adapted instead? We need to figure out what we want before fixing this.

commented

Comment by LadyCailin

CHLog is meant for logging to files, however, we can create a new logging mechanism which is meant for logging to screen and to file, and integrate that with CHLog, then change all uses of CHLog to the new mechanism. Anything that is logged to file should also be logged to screen, I think. The new mechanism would look something like this:

class ItsALoggerButWeNeedABetterName {
    /**
     * This will log the item to screen only if Verbose logging is enabled
     * Target can be null for all items.
     */
    void verboseLogToScreen(String message, Target t);
    void debugLogToScreen(String message, Target t);
    void infoLogToScreen(String message, Target t);
    void etcLogToScreen(String message, Target t);

    /**
     * This will log the item to screen and using CHLog, only if logging is enabled.
     */
    void debugLog(CHLog.Tag tag, String message, Target t);
    void etcLog(CHLog.Tag tag, String message, Target t);
}

CHLog.Tag should probably be changed to an interface, and we allow extensions to @api tag these interfaces, and then we load them. The problem (perhaps) with this is that CHLog uses these tags to generate the config file, which users can then use to configure what levels are in fact logged. The preferences class should be just fine with upgrading preferences if extensions, etc, add new interfaces, so that should not be a problem.

interface LogTag {
    String tagName();
    String documentation();
    com.laytonsmith.core.LogLevel getDefaultLoggingLevel();
}

Then extensions etc that add new tags would do

@api
public enum ExtensionLogTags implements LogTag {
    MY_TAG("my_tag", "What it do yo", LogLevel.ERROR),
    MY_OTHER_TAG("my_other_tag", "What it do yo", LogLevel.INFO);
    /* boilerplate */
}

Alternatively, maybe we just want to combine the two mechanisms, and only have a single logging facility, which logs both to screen and to file, using the CHLog mechanism.

Actually, now that I think about it more, I think it probably makes sense to adapt CHLog instead, and just make Tags extendable, as shown above. (The enum can stay for convenience for core modules, but it too would implement the common interface.)

Also, let's rename it MSLog.

Perhaps we create a new function in MS which is able to re-work the to screen log levels (and hell, file logging too). So, at start up, the screen logging table just is copied from the config, but a call to set_screen_log_level('TAG', 'VERBOSE') would allow a runtime change to the screen logging.

Once we have this design solidified, we can also add a mechanism to change the underlying logging code, so that if a particular backend implementation needs to do the logging differently (bukkit vs spigot vs whatever), then you can install a new underlying mechanism that actually does the appropriate screen logging, whatever that may be. (Default would be to just System.out/err.println, etc.) And that would make it possible to change the underlying mechanism without having to change the code that is using the logger, just need to run a function at startup.

Hopefully this log of my thoughts was verbose enough :D