Review Request: kjs: Implement JSON.parse

[Maks Orlovich]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105056/#review14590
-----------------------------------------------------------



kjs/json_object.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11520>

    What namespace do the symbols for this end up in?
    



kjs/json_object.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11521>

    What happens if the call threw an exception?
    



kjs/json_object.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11522>

    I think it's true, but needs one more proviso: the check on line 89.



kjs/json_object.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11523>

    I think if you don't put in a space between * and /* some Windows compiler will whine.
    (Warnings only, I think, but someone was bothered by it enough in the past to ask us to change a bunch)



kjs/json_object.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11524>

    What happens if toObject throws? (I think it can.. but I might be wrong).
    



kjs/jsonlexer.h
<http://git.reviewboard.kde.org/r/105056/#comment11525>

    explicit



kjs/jsonlexer.h
<http://git.reviewboard.kde.org/r/105056/#comment11526>

    explicit



kjs/jsonlexer.h
<http://git.reviewboard.kde.org/r/105056/#comment11527>

    What's the return value on failure?



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11528>

    static
    



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11529>

    static



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11530>

    static, comment pareameters?
    



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11531>

    I don't think this is needed anymore?
    (Nor the similar one in arrays)



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11532>

    This looks like it will run out of stack space pretty quickly. I think it would be better to instead of firstArrayRun to have something more like what you have for objects. (Or just use propAdded in the same way)
    



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11533>

    So JSON doesn't have 'strings'?
    



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11537>

    Where did the -? go?
    



kjs/jsonlexer.cpp
<http://git.reviewboard.kde.org/r/105056/#comment11538>

    Comment method?


- Maks Orlovich


On May 28, 2012, 7:52 p.m., Bernd Buschinski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105056/
> -----------------------------------------------------------
> 
> (Updated May 28, 2012, 7:52 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> kjs: Implement JSON.parse
> 
> 
> Diffs
> -----
> 
>   kjs/CMakeLists.txt 1188064 
>   kjs/interpreter.cpp cf1acf1 
>   kjs/json_object.h PRE-CREATION 
>   kjs/json_object.cpp PRE-CREATION 
>   kjs/jsonlexer.h PRE-CREATION 
>   kjs/jsonlexer.cpp PRE-CREATION 
>   kjs/libkjs.map e9f679f 
> 
> Diff: http://git.reviewboard.kde.org/r/105056/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bernd Buschinski
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20120610/abf5867c/attachment.htm>