[Webkit-unassigned] [Bug 26083] How to access formatted Java script Console message ?

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26083


Kenneth Rohde Christiansen <kenneth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #72972|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #10 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-11-04 13:00:27 PST ---
(From update of attachment 72972)
View in context: https://bugs.webkit.org/attachment.cgi?id=72972&action=review

Many things to fix... I have up halfway

> WebCore/ChangeLog:5
> +        Fixes bug: https://bugs.webkit.org/show_bug.cgi?id=26083

Please look at other commits on how to format a ChangeLog

> WebCore/page/Console.cpp:149
> +
> +}

why newline here!

> WebCore/page/Console.cpp:200
> +    if (!message.isEmpty())
> +      page->chrome()->client()->addMessageToConsole(JSMessageSource, type, level, message, lastCaller.lineNumber(), lastCaller.sourceURL());

wrong indentation!

> WebCore/page/Console.cpp:505
> +//  get all the arguments in message

We dont do comments like these outside the methods... and comments start with capital letter and ends with a dot!

> WebCore/page/Console.cpp:506
> +void Console::getAllArgumentsAsString(ScriptCallStack* callStack, String& message)

IS this string supposed to not be const?

> WebCore/page/Console.cpp:508
> +{
> + 

No newline needed here

> WebCore/page/Console.cpp:511
> +    String fStr; // formatted string

fStr... that naming is against our style... please consult out style guide and use meaning full variables. Then you wouldn't need useless comments like this one

> WebCore/page/Console.cpp:524
> +        } else {
> +            String argStr;
> +            int idx = 0;
> +            bool repl = false; // should you replace format pattern with value
> +

wow, you misses a } so either your code is wrong or your indentation is very wrong!

> WebCore/page/Console.cpp:529
> +                 // found it

Useless comment.. please remove these

> WebCore/page/Console.cpp:532
> +                 repl = true;

repl? what does that mean? write out variable names

> WebCore/page/Console.cpp:537
> +        if (lastCaller.argumentAt(i).jsValue().isNumber()) { // handle numbers

useless comments... the code pretty much says it

> WebCore/page/Console.cpp:555
> +        if (repl)
> +             fStr.insert(argStr, idx);
> +        else 
> +             fStr += argStr;
> +        }

Look at this code... indendation is wrong or so is your logic! bad, very bad.

> WebCore/page/Console.cpp:567
> +        JSC::ExecState* exec = callStack->globalState();
> +        JSC::JSObject *myObj = jsVal.getObject();

coding style violation and inconsistency

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.