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

feat(console): print proxy details #7139

Merged
merged 7 commits into from
Sep 8, 2020
Merged

feat(console): print proxy details #7139

merged 7 commits into from
Sep 8, 2020

Conversation

uki00a
Copy link
Contributor

@uki00a uki00a commented Aug 21, 2020

Fixes #7096.

Changes

  • Add Deno.core.getProxyDetails to get details of Proxy.
  • Make proxy objects to be properly stringified as follows:
$ ./target/debug/deno
Deno 1.3.0
exit using ctrl+d or close()
> new Proxy([1,2,3], { get() {} })
Proxy [ [ 1, 2, 3 ], { get: [Function: get] } ]
> new Proxy({ a: 1, b: "hello" }, { get() {} })
Proxy [ { a: 1, b: "hello" }, { get: [Function: get] } ]

@bartlomieju
Copy link
Member

@nayeemrmn please review

Copy link
Collaborator

@nayeemrmn nayeemrmn left a comment

Choose a reason for hiding this comment

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

Nice. Node only does this when the showProxy option is given. Our defaults should match Node so to support this we have to add InspectOptions::showProxy. That means 1.4.0.

cli/rt/02_console.js Outdated Show resolved Hide resolved
@uki00a uki00a changed the title console: print proxy details feat(console): print proxy details Aug 24, 2020
Copy link
Collaborator

@nayeemrmn nayeemrmn left a comment

Choose a reason for hiding this comment

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

Also add showProxy: false to DEFAULT_INSPECT_OPTIONS.

Copy link
Collaborator

@nayeemrmn nayeemrmn left a comment

Choose a reason for hiding this comment

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

LGTM (1.4.0)

cli/tests/unit/console_test.ts Outdated Show resolved Hide resolved
@bartlomieju bartlomieju added this to the 1.4.0 milestone Aug 25, 2020
@lucacasonato lucacasonato added the web related to Web APIs label Aug 31, 2020
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @uki00a

@bartlomieju bartlomieju merged commit ac45505 into denoland:master Sep 8, 2020
@uki00a uki00a deleted the issue-7096 branch September 8, 2020 16:37
@uki00a
Copy link
Contributor Author

uki00a commented Sep 8, 2020

@nayeemrmn @bartlomieju Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
web related to Web APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deno is unable to convert Proxy object to a string, thus unable to log
4 participants