tisfeng / Easydict

一个简洁优雅的词典翻译 macOS App。开箱即用,支持离线 OCR 识别,支持有道词典,🍎 苹果系统词典,🍎 苹果系统翻译,OpenAI,Gemini,DeepL,Google,Bing,腾讯,百度,阿里,小牛,彩云和火山翻译。A concise and elegant Dictionary and Translator macOS App for looking up words and translating text.
GNU General Public License v3.0
6.4k stars 325 forks source link

refactor: replace NSLog and print with DDLog #518

Closed phlpsong closed 2 months ago

phlpsong commented 2 months ago

close https://github.com/tisfeng/Easydict/issues/517

tisfeng commented 2 months ago

This PR looks good, but I noticed that you replaced all the NSLog with MMLogInfo, which might not be good.

The project actually has a macro before, #define NSLog(frmt, ...) MMLogVerbose(frmt, ##__VA_ARGS__), https://github.com/tisfeng/Easydict/blob/e5ce53bc20ad6b34d04dff62a05fa2d6a361818c/Easydict/App/PrefixHeader.pch#L9 which has a very similar function, but this causes two problems:

  1. sometimes we only want to print the log to the console, just for our debugging use, we don't want to write to the user log file.
  2. sometimes the debug log content is very large, if all write to the log file, may lead to a large disk occupation.

Considering the above reasons, so I commented out this macro.

I would like you to add a MMLog to replace the system NSLog for console log printing only, but with better performance and log hierarchy management.

phlpsong commented 2 months ago

We have this definition in MMLog which we could use to constraint log output in debug/release.

#if DEBUG
DDLogLevel MMDefaultLogLevel = DDLogLevelAll;
#else
DDLogLevel MMDefaultLogLevel = DDLogLevelInfo;
#endif

After the change [ddlog addLogger:fileLogger withLevel:MMDefaultLogLevel];, it will only output Error, warning and info logs to the file in release mode and all logs in debug mode.

Additionally, I have changed some of the info logs that I think are not very necessary to verbose and the file size configuration.

typedef NS_ENUM(NSUInteger, DDLogLevel){
    /**
     *  No logs
     */
    DDLogLevelOff       = 0,

    /**
     *  Error logs only
     */
    DDLogLevelError     = (DDLogFlagError),

    /**
     *  Error and warning logs
     */
    DDLogLevelWarning   = (DDLogLevelError   | DDLogFlagWarning),

    /**
     *  Error, warning and info logs
     */
    DDLogLevelInfo      = (DDLogLevelWarning | DDLogFlagInfo),

    /**
     *  Error, warning, info and debug logs
     */
    DDLogLevelDebug     = (DDLogLevelInfo    | DDLogFlagDebug),

    /**
     *  Error, warning, info, debug and verbose logs
     */
    DDLogLevelVerbose   = (DDLogLevelDebug   | DDLogFlagVerbose),

    /**
     *  All logs (1...11111)
     */
    DDLogLevelAll       = NSUIntegerMax
};

Please let me know what you think.

tisfeng commented 2 months ago

I added MMLog for printing logs in debug mode without writing to the user log file, for logs that need to be written to the user file I can use MMLogInfo.

#define MMLog(frmt, ...) LOG_MAYBE_TO_DDLOG([MMManagerForLog sharedDDLog], MMDefaultLogAsyncEnabled, MMDefaultLogLevel, DDLogFlagVerbose, 0, nil, __PRETTY_FUNCTION__, frmt, ##__VA_ARGS__)

#define MMLogInfo(frmt, ...) LOG_MAYBE_TO_DDLOG([MMManagerForLog sharedDDLog], MMDefaultLogAsyncEnabled, MMDefaultLogLevel, DDLogFlagInfo, 0, nil, __PRETTY_FUNCTION__, frmt, ##__VA_ARGS__)
tisfeng commented 2 months ago

I have a question as to why the Type identifier for MMLogError in the Xcode console is a yellow warning ⚠️ instead of red ❗.

Also, the Type identifiers of MMLogInfo and MMLogWarn are the same, both are blue, which is strange.

image image
tisfeng commented 2 months ago

I can filter out logs of type MMLogError using type=error, but when using type=info, it includes both MMLogInfo and MMLogWarn, which is strange and not in line with our design expectations.

I'm not very familiar with the logging system, so please let me know if I've misunderstood anything.

image image
tisfeng commented 2 months ago

ok, I see. DDLogFlagWarning and DDLogFlagInfo are actually the same type at the system level, they're just categorized differently by CocoaLumberjack.

- (void)logMessage:(DDLogMessage *)logMessage {
    // Skip captured log messages
    if ([logMessage->_fileName isEqualToString:@"DDASLLogCapture"]) {
        return;
    }

    if (@available(iOS 10.0, macOS 10.12, tvOS 10.0, watchOS 3.0, *)) {
        NSString * message = _logFormatter ? [_logFormatter formatLogMessage:logMessage] : logMessage->_message;
        if (message != nil) {
            const char *msg = [message UTF8String];
            __auto_type logger = [self logger];
            switch (logMessage->_flag) {
                case DDLogFlagError  :
                    os_log_error(logger, "%{public}s", msg);
                    break;
                case DDLogFlagWarning:
                case DDLogFlagInfo   :
                    os_log_info(logger, "%{public}s", msg);
                    break;
                case DDLogFlagDebug  :
                case DDLogFlagVerbose:
                default              :
                    os_log_debug(logger, "%{public}s", msg);
                    break;
            }
        }
    }
}