Skip to content

Conversation

@pfalcon
Copy link
Contributor

@pfalcon pfalcon commented Jul 7, 2016

Using jerry_eval() instead of jerry_run_simple() allows to store (and then access)
variables in a global environment. E.g. following now works:

a = 1
print(a)

JerryScript-DCO-1.0-Signed-off-by: Paul Sokolovsky paul.sokolovsky@linaro.org

@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 7, 2016

This follows Unix port (main-unix.c) and API example doc.

@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 7, 2016

@sergioamr, @poussa: FYI.

@zherczeg
Copy link
Member

zherczeg commented Jul 8, 2016

Sounds like a good idea. The run_simple initializes and shuts down the engine, so it cannot return with any meaningful value, since jerry heap has been freed already.

@zherczeg
Copy link
Member

zherczeg commented Jul 8, 2016

LGTM

1 similar comment
@poussa
Copy link
Contributor

poussa commented Jul 8, 2016

LGTM

@LaszloLango
Copy link
Contributor

This won't work after #1177
@pfalcon, would you like to land it now or wait for the API update first?

@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 8, 2016

@LaszloLango : If #1177 is close to be merged (it seems to be), I guess it makes sense to land it first, then I'll update this patch. Thanks.

@pfalcon pfalcon changed the title targets/zephyr: Use jerry_eval() to provider better REPL (interactive prompt). targets/zephyr: Use jerry_eval() to provide better REPL (interactive prompt). Jul 8, 2016
@sergioamr
Copy link
Contributor

LGTM. I would like to be able to have the Travis integration working, so changes on the API get highlighted and do not break the build.

The situation is that @Akromx16 will go back to university really soon and i am not sure if he would be able to finish his integration on #1144

Could someone with travis experience help on that PR?

@LaszloLango
Copy link
Contributor

#1177 is merged. docs/02.API-EXAMPLES.md is up to date, but docs/02.API-REFERENCE.md is not. Let me know, if you need any help with the update.

@LaszloLango LaszloLango added the jerry-port Related to the port API or the default port implementation label Jul 11, 2016
@pfalcon pfalcon changed the title targets/zephyr: Use jerry_eval() to provide better REPL (interactive prompt). targets/zephyr: Update to new Jerry API and improve REPL interaction. Jul 11, 2016
@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 11, 2016

@LaszloLango : Thanks, refactored to the new API, rebased to master as of now.

@zherczeg
Copy link
Member

LGTM

jerry_init (JERRY_INIT_EMPTY);
shell_register_app_cmd_handler (shell_cmd_handler);
shell_init ("js> ", commands);
} /* main */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jerry_cleanup should be called before the end of main

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LaszloLango : That's not the case with Zephyr. Generally, a typical embedded app would work as an infinite loop (it "exits" via hardware reset), and specifically with Zephyr, shell_init() just sets up background task and exit, all actual input passed to callbacks at later time. So, it's not ok to deinitialize Jerry after call to shell_init(), and per above, no need to deinitialize at all. Hope that makes sense. I'm adding a comment to the patch with this info.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LaszloLango

shell_init will spawn a fiber that performs the shell task.
task_fiber_start(stack, STACKSIZE, shell, 0, 0, 7, 0);

If you clean zephyr there it will just crash.

With the change of running code on eval i think pfalcon is right on not cleaning up after execution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfalcon, @sergioamr, OK, thanks for the explanation.

@LaszloLango
Copy link
Contributor

LGTM after jerry_cleanup call is added.

@sergioamr
Copy link
Contributor

lgtm

Various calls updated to use API from jerry-api.h (to-be JerryScript 1.0
API).

Use jerry_eval() instead of jerry_run_simple(), as it allow to store (and
then access) variables in a global environment. E.g. following now works
(entered and executed as two separate lines):

    a = 1
    print(a)

JerryScript-DCO-1.0-Signed-off-by: Paul Sokolovsky paul.sokolovsky@linaro.org
@pfalcon
Copy link
Contributor Author

pfalcon commented Jul 12, 2016

Added comment to the source why jerry_cleanup() is not called, to address @LaszloLango's comment above.

@LaszloLango
Copy link
Contributor

LGTM

@LaszloLango LaszloLango merged commit 6b60e22 into jerryscript-project:master Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jerry-port Related to the port API or the default port implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants