Summary: | Calling console.{log, info, error, ...} should not require this === console | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Victor Costan <costan> | ||||||
Component: | WebCore JavaScript | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED DUPLICATE | ||||||||
Severity: | Normal | CC: | ap, commit-queue, esprehn+autocc, graouts, joepeck, ojan.autocc, timothy | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Victor Costan
2013-04-10 19:07:47 PDT
Created attachment 197464 [details]
Patch
Attachment 197464 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/dom/Window/console-unbound-functions-expected.txt', u'LayoutTests/fast/dom/Window/console-unbound-functions.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/IDLAttributes.txt', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/page/Console.cpp', u'Source/WebCore/page/Console.h', u'Source/WebCore/page/Console.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:188: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 1 in 18 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 197464 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197464&action=review > Source/WebCore/page/Console.idl:62 > - [CallWith=ScriptArguments|ScriptState] void clear(); > + [CallWith=ScriptArguments|ScriptState] static void clear(); Why is this different from the others? Was this an earlier syntax you had experimented with? Created attachment 197499 [details]
Patch
Darin, thank you so much for catching that! Yes, I tried static first, hoping it would work. I'm really sorry about this omission, I was so happy when I got the IDL generator working that I forgot to double-check my Console changes. :( Also, should I worry about the style error in the generated code? Attachment 197499 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/dom/Window/console-unbound-functions-expected.txt', u'LayoutTests/fast/dom/Window/console-unbound-functions.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/IDLAttributes.txt', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/page/Console.cpp', u'Source/WebCore/page/Console.h', u'Source/WebCore/page/Console.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:188: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 1 in 18 files
If any of these errors are false positives, please file a bug against check-webkit-style.
If you agree, I think that it would be better to dupe this to older bug, and to move the patch there. Thank you for pointing this out, Alexey! Should I add the description of my patch as a comment in #20141? *** This bug has been marked as a duplicate of bug 20141 *** > Should I add the description of my patch as a comment in #20141? Sure, bug 20141 should have enough information to learn what's going on. |