Summary: | Make it possible to enable log channels in WebKitTestRunner and DumpRenderTree | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Component: | Tools / Tests | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | sam, simon.fraser, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2021-12-20 13:31:47 PST
Created attachment 447629 [details]
Patch
Comment on attachment 447629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447629&action=review > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:124 > + [[NSUserDefaults standardUserDefaults] setValue:[NSString stringWithUTF8String:options.webCoreLogChannels.c_str()] forKey:@"WebKitLogging"]; This should be `options.webKitLogChannels.c_str()`. Created attachment 447634 [details]
Patch
Created attachment 447639 [details]
Patch
Comment on attachment 447629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447629&action=review > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1008 > + {"WebCoreLogging", required_argument, nullptr, 'W'}, The other options use dashes between words. For consistency, how about -webcore-logging? > Tools/WebKitTestRunner/Options.cpp:155 > +static bool handleOptionLogChannels(Options& options, const char* option, const char* channels) Can you move this above the `handleOptionUnmatched` function? I think It makes sense to keep that one last. > Tools/WebKitTestRunner/Options.cpp:194 > + optionList.append(Option("--WebCoreLogging", "Enable WebCore log channels", handleOptionLogChannels, true)); > + optionList.append(Option("--WebKitLogging", "Enable WebKit log channels", handleOptionLogChannels, true)); Same comment as about. Can we keep these consistent as well? Also, I would recommend just using two different functions rather than reparsing. |