Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support console API in shell and debugger #1012

Closed
MaxisTekfield opened this issue Aug 28, 2021 · 10 comments
Closed

Support console API in shell and debugger #1012

MaxisTekfield opened this issue Aug 28, 2021 · 10 comments
Labels
feature Issues considered a new feature

Comments

@MaxisTekfield
Copy link
Contributor

As I'm using java -jar rhino-1.7.13.jar to run rhino as a shell, or java -cp rhino-1.7.13.jar org.mozilla.javascript.tools.debugger.Main to run rhino debugger, I'd like to write console.log(...) or something like this to check values of variables.
The console API is very popular and widely used in other JS engines, eg nodejs, firefox, chrome. One of the very useful features is printf-like format, which print API does not support. Especially the %o and %O specifier can check objects very easily.
Expecting rhino can support console API in shell and debugger so that we can use same statements in all these popular js engines.

For reference:

@MaxisTekfield MaxisTekfield changed the title Support console API in console Support console API in shell and debugger Aug 28, 2021
@p-bakker p-bakker added the feature Issues considered a new feature label Aug 28, 2021
@p-bakker
Copy link
Collaborator

p-bakker commented Aug 28, 2021

See relevant discussions comments: #972 (reply in thread) and #972 (comment)

And see MDN Console and WhatWG Console living standard

As Console is not part of the EcmaScript standard, it should not be enabled by default, but an optional opt-in via a Feature Flag.

And we'd have to decide whether to enable by default in the Shell or not. If we do, should we disable print (by default)? And maybe only from v2.0.0 onwards?

And as for output in the Shell: in Chrome when you output an Object in the debugger console, the object is interactive. Current shell just outputs [object Object], which isn't that usefull imho. Similar behavior as in the Chrome debugger Console would be great, not sure if that is achievable.

As a sidenote to this: Rhino supports adding (an old version of) JLine to get a better commandline experience. Not well documented (at all?) and I wasn't able to (quickly) get it going, but maybe that can aid in provising a better DUX somehow when outputting stuff via console.xxx()

@MaxisTekfield
Copy link
Contributor Author

MaxisTekfield commented Aug 30, 2021

I agree that console should not be enabled by default. Enabling console as an optional feature is a good idea.
I think it should be enabled in the shell by default, comparing to nodejs console and the DevTools console of chrome. But print should not disabled immediatly once console is enabled. These two APIs should coexist for a while for compatible reasons. Maybe removing print in v1.8 or v2.0 is good and should be listed in breaking changes.

In chrome, I can print an object by console.log("%o", obj), assuming obj refers to an object. Instead, I have to write print(JSON.stringify(obj)) which is longer and not so easy to get result like that.
As for interactivity, I don't think it's the key feature of console. Because it can be only used in GUI applications, not for text-interacting shells. Check nodejs.
On the other hand I think the printf-like formatting of console is very helpful and this is the key feature.

@tonygermano
Copy link
Contributor

I would say yes to enabling console by default in the shell, but I don't see any reason to disable print.

Interactive object printing I would think would be enabled by the UI itself. Either by having different implementations of the console object itself, or the console implementation would have to take some kind of output processor that could convert objects to strings for the cli shell or interactive trees for a gui interface.

@gbrail
Copy link
Collaborator

gbrail commented Aug 30, 2021

One way to handle this would be to implement "console" in the "Global" object, which is used by the Shell, but which can be used in other contexts as well -- a lot of Rhino code seems to depend on it, actually.

Adding console to that object sounds like a great project for someone new to the project and shouldn't require a huge amount of context to implement. I hope someone is interested in working on it!

@p-bakker
Copy link
Collaborator

p-bakker commented Aug 31, 2021

Not sure if implementing the console api in the Global object is the best way forward: I rather see it as a separate thing, that is/can be added to the Shell by default, but could also be added to custom embeddings if the embedder decides to do so, without being forced to use the Globals scope that comes with a whole bunch of other (potentially undesired) stuff as well.

This also brings up another (design) question: as an embedder, you want the console api, but you might want to control where the actual output goes

@gbrail
Copy link
Collaborator

gbrail commented Aug 31, 2021 via email

@p-bakker
Copy link
Collaborator

Within ScriptRuntime.initSafeStandardObjects we seem to use two different approaches:

  1. class init method, like NativeMath.init(scope, sealed);
  2. Lazy Constructors, like new LazilyLoadedCtor(scope, "ArrayBuffer", "org.mozilla.javascript.typedarrays.NativeArrayBuffer", sealed, true);

With some things like E4x behind a fature flag.

I think either of the two approaches above would be fine for embedders and would also be easy to add stuff to Globals for the shell.

IMHO we don't need feature flags for things like settimeout, console and whats not. Better document all the optional things that are available in our new documentation

@p-bakker
Copy link
Collaborator

Some discussion going on inside the PR about how to make the implement versatile, so embedded can use it as well.

Input appreciated

@p-bakker
Copy link
Collaborator

Note: once the console functionality is merged, update the 'Update core-js-compat' section in RELEASE-STEPS.md to not include the conversion of console.log > print (see #1164)

@p-bakker
Copy link
Collaborator

p-bakker commented Apr 4, 2022

Closed via b05aa96

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Issues considered a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants