Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(654)

Issue 7519: Added first shot at a development shell (Closed)

Created:
12 years, 2 months ago by Christian Plesner Hansen
Modified:
9 years, 7 months ago
Reviewers:
Kasper Lund
CC:
v8-dev
Visibility:
Public.

Description

Added development shell (d8) including readline support, counters and completion.

Patch Set 1 #

Patch Set 2 : Factored js code out of d8.cc. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+743 lines, -28 lines) Patch
M SConstruct View 7 chunks +31 lines, -5 lines 0 comments Download
M src/SConscript View 1 3 chunks +20 lines, -4 lines 0 comments Download
A src/d8.h View 1 chunk +110 lines, -0 lines 1 comment Download
A src/d8.cc View 1 1 chunk +347 lines, -0 lines 0 comments Download
A src/d8.js View 1 chunk +74 lines, -0 lines 5 comments Download
A src/d8-readline.cc View 1 chunk +118 lines, -0 lines 0 comments Download
M src/flag-definitions.h View 2 chunks +4 lines, -1 line 0 comments Download
M src/natives.h View 2 chunks +8 lines, -1 line 0 comments Download
M src/smart-pointer.h View 2 chunks +6 lines, -1 line 0 comments Download
M tools/js2c.py View 2 chunks +25 lines, -16 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Christian Plesner Hansen
12 years, 2 months ago (2008-10-20 15:05:03 UTC) #1
Christian Plesner Hansen
I've updated this changelist. Now the javascript code from within d8.cc is factored out into ...
12 years, 2 months ago (2008-10-21 07:26:31 UTC) #2
Kasper Lund
12 years, 2 months ago (2008-10-21 08:26:56 UTC) #3
LGTM. I guess the next step is to start using d8 to run our tests instead of the
shell sample. Is there anything we could remove from the shell sample once d8 is
alive and kicking?

http://codereview.chromium.org/7519/diff/401/605
File src/d8.h (right):

http://codereview.chromium.org/7519/diff/401/605#newcode108
Line 108: }  // v8
Maybe add some more whitespace here. There's plenty in other parts of the the
file.

http://codereview.chromium.org/7519/diff/401/606
File src/d8.js (right):

http://codereview.chromium.org/7519/diff/401/606#newcode33
Line 33: for (var i = 0; i < str.length; i++) {
How about using substring and string equality check?

http://codereview.chromium.org/7519/diff/401/606#newcode42
Line 42: return undefined;
I don't trust undefined not to be overwritten by users. I would just use
'return' - or maybe 'return void 0'.

http://codereview.chromium.org/7519/diff/401/606#newcode54
Line 54: for (var i in parts) {
This looks weird. Do you really want all the keys (0, 1, ...) as strings? Why
not use a for (var i = 0; i < parts.length; i++)?

http://codereview.chromium.org/7519/diff/401/606#newcode63
Line 63: while (current !== undefined) {
I would use a typeof check against 'undefined' here.

http://codereview.chromium.org/7519/diff/401/606#newcode66
Line 66: for (var i in properties) {
Do you really want to use for-in here too?

Powered by Google App Engine
This is Rietveld 408576698