178276 2017-10-13 11:49:13 -0700 [WPE][GTK] Unify/simplify/improve PAL::logLevelString, WebCore::logLevelString, and WebKit::logLevelString 2025-07-09 09:11:39 -0700 1 1 1 Unclassified WebKit Platform Other Unspecified Unspecified RESOLVED FIXED P2 Normal --- 178263 252558 295481 1 aperez webkit-unassigned annulen cgarcia clopez oldest_to_newest 1360136 0 aperez 2017-10-13 11:49:13 -0700 We currently have: - PAL::logLevelString (Source/WebCore/PAL/pal/unix/LoggingUnix.cpp) - WebCore::logLevelString (Source/WebCore/platform/unix/LoggingUnix.cpp) - WebKit::logLevelString (Source/WebKit/Platform/unix/LoggingUnix.cpp) Essentially, the three do the same, which is: 1. Picking the value from the WEBKIT_DEBUG environment variable. 2. If the variable is not present, return an empty string. 3. If the variable is defined: 3.a. Warn with “WTFLogAlways” that some logging is only available in debug builds. 3.b. Prepend “NotYetImplemented,” to the obtained value (except for WebKit::logLevelString). 3.c. Return the value. During a discussion with Konstantin Tokarev and Carlos López we came up with the following possible improvements/doubts: - Unifying all of them, by making both WebCore::logLevelString and WebKit::logLevelString just call PAL::logLevelString. - Removing the message emitted with WTFLogAlways(). - Does it make sense to always prepend “NotYetImplemented,”? Why was it not being done for the WebKit version? 2127916 1 aperez 2025-07-04 14:05:58 -0700 (In reply to Adrian Perez from comment #0) > We currently have: > > - PAL::logLevelString (Source/WebCore/PAL/pal/unix/LoggingUnix.cpp) This instance still has some custom code, it should be changed to call WTF::logLevelString() -- read on below. > - WebCore::logLevelString (Source/WebCore/platform/unix/LoggingUnix.cpp) > - WebKit::logLevelString (Source/WebKit/Platform/unix/LoggingUnix.cpp) These two instances have been changed to call WTF::logLevelString() in 260601@main > Essentially, the three do the same, which is: > > 1. Picking the value from the WEBKIT_DEBUG environment variable. > 2. If the variable is not present, return an empty string. > 3. If the variable is defined: > 3.a. Warn with “WTFLogAlways” that some logging is only available > in debug builds. > 3.b. Prepend “NotYetImplemented,” to the obtained value (except > for WebKit::logLevelString). > 3.c. Return the value. > > During a discussion with Konstantin Tokarev and Carlos López we came up > with the following possible improvements/doubts: > > - Unifying all of them, by making both WebCore::logLevelString and > WebKit::logLevelString just call PAL::logLevelString. This was partially done in 260601@main -- except for the PAL one, as outlined above. > - Removing the message emitted with WTFLogAlways(). > - Does it make sense to always prepend “NotYetImplemented,”? Why was it > not being done for the WebKit version? Still no idea about these two. 2127986 2 aperez 2025-07-05 14:14:10 -0700 (In reply to Adrian Perez from comment #1) > (In reply to Adrian Perez from comment #0) > > We currently have: > > > > - PAL::logLevelString (Source/WebCore/PAL/pal/unix/LoggingUnix.cpp) > > This instance still has some custom code, it should be changed to call > WTF::logLevelString() -- read on below. Actually, nothing seems to call PAL::logLevelString() presently. I am looking into removing it in bug #295481 > > - WebCore::logLevelString (Source/WebCore/platform/unix/LoggingUnix.cpp) > > - WebKit::logLevelString (Source/WebKit/Platform/unix/LoggingUnix.cpp) > > These two instances have been changed to call WTF::logLevelString() in > 260601@main And because those two have been simplified already, once the patch for #295481 lands, we can close this one as well. 2128710 3 aperez 2025-07-09 09:11:39 -0700 (In reply to Adrian Perez from comment #2) > (In reply to Adrian Perez from comment #1) > > (In reply to Adrian Perez from comment #0) > > > We currently have: > > > > > > - PAL::logLevelString (Source/WebCore/PAL/pal/unix/LoggingUnix.cpp) > > > > This instance still has some custom code, it should be changed to call > > WTF::logLevelString() -- read on below. > > Actually, nothing seems to call PAL::logLevelString() presently. I am > looking into removing it in bug #295481 The patch for that landed, so I think we can consider this issue solved.