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

refactor: Use Display trait on InputValue for logging #1988

Closed
vezenovm opened this issue Jul 20, 2023 · 1 comment · Fixed by #1990
Closed

refactor: Use Display trait on InputValue for logging #1988

vezenovm opened this issue Jul 20, 2023 · 1 comment · Fixed by #1990
Assignees
Labels
enhancement New feature or request nargo Noir's CLI development tool refactor

Comments

@vezenovm
Copy link
Contributor

Problem

Reference this comment in this discussion: #1917 (comment)

The JsonTypes enum is being used inside the nargo crate in order to display InputValue types that have been decoded from foreign calls. We want to restrict the JsonTypes to just the noirc_abi crate.

Happy Case

Decoding a list of field elements to an InputValue and then converting it into a JsonType requires an AbiType. We should make a wrapper around InputValue purely for display called InputValueDisplayWrapper or something similar. This wrapper will contain the InputValue to display and the AbiType. The wrapper's Display implementation will then do the conversion into JsonTypes inside the noirc_abi crate so that we don't expose format types outside of the Abi crate.

Alternatives Considered

N/A

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@vezenovm
Copy link
Contributor Author

PR #1990 is up for this

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request nargo Noir's CLI development tool refactor
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants