gmane.comp.kde.devel.core

Review Request: kjs: Implement JSON.stringify

Bernd Buschinski — 2012-05-25T20:19:54

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

Review request for kdelibs.


Description
-------

kjs: Implement JSON.stringify

patch needs https://git.reviewboard.kde.org/r/105056/ (JSON.parse)


Diffs
-----

  kjs/CMakeLists.txt 1188064 
  kjs/CommonIdentifiers.h 8ee40e8 
  kjs/json_object.h PRE-CREATION 
  kjs/json_object.cpp PRE-CREATION 
  kjs/jsonstringify.h PRE-CREATION 
  kjs/jsonstringify.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/105057/diff/


Testing
-------


Thanks,

Bernd Buschinski

Review Request: kjs: Implement JSON.parse

Bernd Buschinski — 2012-05-25T20:19:54

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

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

Re: QtScript considered dangerous

Stephen Kelly — 2012-05-25T19:42:50


Nope. QtScript might be 'done' - meaning no one is working on it and there's 
no replacement. The v8 stuff is not public API like QtScript is, so it is 
not a replacement.


Yes. That's how QtDeclarative works. As it's internal, non-public API, that 
doesn't really matter much.

Maybe a public API will be created to replace QtScript eventually though...

Thanks,

Steve.

Re: QtScript considered dangerous

Aaron J. Seigo — 2012-05-25T12:52:44

ime, Kent tends to be focused on the better future rather than the pragmatic 
present while tending to ignore use cases that his team doesn't have 
themselves. this can make using things that comes from his team at times 
rather difficult. we've gone through these sorts of issues a couple times 
already in the past.

(i love his excuse for not including a QScriptContext style API in QJS, given 
that it is indeed highly useful: "it's an implementation detail". it is never 
useful to be able to introspect into the call stack, right? *sigh* 
QScriptContext itself does expose too many implementation details, but the 
idea of QScriptContext is rather good. it would be nice to see it conceptually 
improved rather than dumped becase of problems with the current 
implementation.)

i find that usually pressing the issue, reiterating the use cases, finding 
others with the use cases and producing patches (ignoring the "we might not 
even accept changes" sillyness) tends to help.

there's no reason to throw the baby o

Re: QtScript considered dangerous

Thomas Lübking — 2012-05-25T11:52:49

2012/5/25, Martin Sandsmark <sandsmark< at >iskrembilen.com>:

It likely asserts in the casting comparism.

tl;dr : the code looks fragile.

ntl;wr

The function seems supposed to subtract two 64bit pointers into a
32bit one but whether the issue is that the the parameters fail* or
the reinterpret_cast<intptr_t> stuff is broken in libc cannot be said
w/o adding more detailed debgging code and testing the thing.

if the offset would indeed be < 2^31 (2^32 for uints) but the intptr_t
casting stuff fails, one could try to simply subtract the void*'s

I don't know what this is ultimately supposed to do and be good for,
but hashing the 64bits into 32bits is likely far more robust (and the
application/class could also just reserve memory in the lower 32bit
range for such specific purposes)

*ie "from - to" results in a value beyond 32bit limits, what can
easily happen if you just subtract one random 64bit value from
another, so the higher** in virtual memory the pointers are, the more
likely a "naive" approach on this

KDE SC 4.9 Features

Allen Winter — 2012-05-25T10:59:50

Howdy,

If you have implemented an important new feature to KDE SC 4.9
please remember to add it to the feature list (wiki) at:

http://techbase.kde.org/Schedules/KDE4/4.9_Feature_Plan

Or, if you already have feature(s) on the list please update the status as appropriate.

Do NOT add new features that you plan for KDE SC 4.10.
We will open a wiki for the 4.10 features in a few weeks.


PS: You can send me the info and I will add it for you.
I need the project name and 1-2 line description.

-Allen

Re: QtScript considered dangerous

Martin Sandsmark — 2012-05-25T10:32:07

Isn't the problem that the pointer size it uses is too small?

Re: QtScript considered dangerous

Christoph Cullmann — 2012-05-25T09:55:08

Still, a fix for QtScript would be the nicest solution or a port to the "new" 
engine provided in Qt5, as I understood, there QtScript is anyway "deprecated" in favour
of the V8 based new variant?

But does the new engine provide the same nice features to easily wrap your objects and expose a custom API?
That was the major reason we switched over from KJS to QtScript.

Greetings
Christoph

Re: QtScript considered dangerous

Dominik Haumann — 2012-05-25T09:18:31

And it's unclear, whether a jsc update would fix the issue, btw :-)

Dominik

Re: QtScript considered dangerous

Dominik Haumann — 2012-05-25T09:14:17

That would certainly be the correct solution.

On [1], the Kent Hansen says:
  We maintained a set of patches on top of the upstream JavaScriptCore
  to be able to support the full QtScript API, but the maintenance cost
  was too high. The patches relied on JSC internals that are subject to
  change at any time. The public JSC C API wasn't extensive enough to
  be able to implement the QtScript API on top of. [...]
So it's probably non-trivial to create this patch (haven't looked into
it, though), a dead end as it's Qt4, and unclear, whether such a patch
would be accepted at all in the Qt 4.x line, given that the focus is
on Qt5 now.

[1] https://bugreports.qt-project.org/browse/QTBUG-16478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#issue-tabs

Greetings,
Dominik

Re: Review Request: Remove additional directories from shortcuts scheme export path

Michael Pyne — 2012-05-24T23:44:25

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

Ship it!


In response to the ping, I like the concept of the bugfix and the patch appears sound. I haven't tested it though but if no one has complained in patch review yet then it's probably safe to commit. ;)

- Michael Pyne


On May 24, 2012, 10:01 p.m., Burkhard Lück wrote:

Re: Review Request: QSslConfiguration set/get functions for KTcpSocket

Dawit Alemayehu — 2012-05-24T22:52:43

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

(Updated May 24, 2012, 10:52 p.m.)


Review request for kdelibs and Thiago Macieira.


Changes
-------

If there is no objection on this new API addition by Sun May 27, I am going to push this into 4.8 branch on Monday so that the TCPSlaveBase bug fix can make it into 4.8.4 release.


Description
-------

The attached patch adds QSslConfiguration getter/setter functions to KTcpSocket. That way users of KTcpSocket can set their own Ssl configuration information on the socket. 

Note that a patch that addresses a current SSL related bug in TCPSlaveBase requires this change. See https://git.reviewboard.kde.org/r/103610/.


Diffs
-----

  kdecore/network/ktcpsocket.h 982e5b1 
  kdecore/network/ktcpsocket.cpp 57d54a9 

Diff: http://git.reviewboard.kde.org/r/104958/diff/


Testing
-------


Than

Re: Review Request: Remove additional directories from shortcuts scheme export path

David Faure — 2012-05-24T22:01:17

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

(Updated May 24, 2012, 10:01 p.m.)


Review request for kdelibs and Andreas Pakulat.


Changes
-------

Seems to be apaku's code.


Description
-------

The Configure Shortcuts dialog has an Action to export a scheme 
(Details->More Actions->Export Scheme)

Using this action the user is asked for a export location and has to select a directory.
Then the current scheme 'schemename' in application 'appname' is exported to a file 
named appnameschemenameshortcuts.rc.

But this file is not saved in the selected directory as Joe User would expect, but in 
shortcuts/share/apps/appname/ below the selected folder.

This patch removes the additional directories shortcuts/share/apps/appname/ from 
the export path to make it easier for the user to find the scheme file and move/copy 
it via command li

Re: Review Request: Apper on kdereview

Matthias Klumpp — 2012-05-24T20:48:32

Hi!

2012/5/24 Burkhard Lück <lueck< at >hube-lueck.de>:
Too early? (I thought too late :P)
Changed as you suggested.

Great!
Many thanks! :)

Re: Review Request: Remove additional directories from shortcuts scheme export path

Burkhard Lück — 2012-05-24T19:34:33

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


Ping?

- Burkhard Lück


On May 18, 2012, 7:11 a.m., Burkhard Lück wrote:

Re: Review Request: Do not restore previously empty location bar URL

Commit Hook — 2012-05-24T17:36:33

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


This review has been submitted with commit 515aec38fb8e5dce231709de326140040097a504 by Dawit Alemayehu to branch master.

- Commit Hook


On May 18, 2012, 7:54 a.m., Dawit Alemayehu wrote:

Re: Review Request: Do not restore previously empty location bar URL

Commit Hook — 2012-05-24T17:35:12

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


This review has been submitted with commit 98f63a671d0b8b76d69c7d4b5fc06c5837408672 by Dawit Alemayehu to branch KDE/4.8.

- Commit Hook


On May 18, 2012, 7:54 a.m., Dawit Alemayehu wrote:

Re: QtScript considered dangerous

Alexander Neundorf — 2012-05-24T17:34:14

To me, "Since this issue renders KDevelop unusable (crashes with stack 
corruption or assertion every few keypresses that issue a run of the indenter 
script)" sounds like a serious bug, which needs to be fixed in some way.

Alex

Re: Review Request: Do not restore previously empty location bar URL

David Faure — 2012-05-24T16:53:35



ESC restores the url when typing, indeed. But not in the case of URLs not handled by konqueror.

Anyway, it seems we agree, let's remove the call.


- David


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


On May 18, 2012, 7:54 a.m., Dawit Alemayehu wrote:

Re: Review Request: Apper on kdereview

Burkhard Lück — 2012-05-24T16:44:12

Am Donnerstag, 24. Mai 2012, 17:07:20 schrieb Matthias Klumpp:
Afaik the insertCatalog() call is done too early and won't work.

KAboutData Class Reference says about the second parameter of the constructor:
catalogName
The translation catalog name; if null or empty, the appName will be used. You 
may want the catalog name to differ from program name, for example, when you 
want to group translations of several smaller utilities under the same 
catalog.

So I'd use: 
KAboutData aboutData("appsetup-kde", "apper", ...

Yuri added apper/doc to scripty's workflow, man pages will be processed 
tomorrow morning.

Thanks.

Re: Review Request: Do not restore previously empty location bar URL

Dawit Alemayehu — 2012-05-24T16:34:40



I do not have a problem with doing that especially since pressing ESC while in the location bar will automatically restore the URL of the current content. Do you want me to completely remove the setLocationBarURL call then ?


- Dawit


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


On May 18, 2012, 7:54 a.m., Dawit Alemayehu wrote: