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

Adds custom inspect method for URL #3241

Merged
merged 2 commits into from
Oct 31, 2019

Conversation

svnv
Copy link
Contributor

@svnv svnv commented Oct 29, 2019

Adds a custom inspection function for the URL class so that some hardcoded properties will always be listed in full.

Eg.

> new URL("http:https://example.com/?");
URL { href: "http:https://example.com/?", origin: "http:https://example.com", protocol: "http:", username: "", password: "", host: "example.com", hostname: "example.com", port: "", pathname: "/", hash: "", search: "?" }

Closes #3124

Using custom inspection function implemented in #2791

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2019

CLA assistant check
All committers have signed the CLA.

@svnv
Copy link
Contributor Author

svnv commented Oct 30, 2019

Another and slightly more generic approach here would be to add something like this

[customInspectProperties]:["href", "origin" ....]

to the URL class and handle the presentation of those in the console.ts file instead.

ry
ry approved these changes Oct 30, 2019
cli/js/url_test.ts Outdated Show resolved Hide resolved
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

@kitsonk please review if you have a chance.

Copy link
Contributor

@kitsonk kitsonk left a comment

Choose a reason for hiding this comment

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

One thought about the test, but otherwise looks good.

cli/js/url_test.ts Outdated Show resolved Hide resolved
"hash",
"search"
];
const objectString = keys
Copy link
Contributor

Choose a reason for hiding this comment

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

Fine for now, but longer term we should think about exporting helper function from console.ts to do consistent formatting and coloring of the output, but that is bigger scope than this PR.

@svnv
Copy link
Contributor Author

svnv commented Oct 31, 2019

Updated pull request after code review.

Copy link
Member

@ry ry 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!

@ry ry merged commit d7a5aed into denoland:master Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URL interface display
4 participants