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

[solcjs] --pretty-json and --verbose options #534

Merged
merged 2 commits into from
Sep 22, 2021

Conversation

cameel
Copy link
Member

@cameel cameel commented Jul 9, 2021

Somewhat related to ethereum/solidity#11583.

This PR adds two options

  • --pretty-print - we already have that in solc and soon it will work for Standard jSON too (Make --pretty-json work with Standard JSON output solidity#11639).
  • --verbose - prints extra info, most importantly the Standard JSON input to the compiler. It's most useful when not using --standard-json option because then it's the script that constructs the input JSON. I wanted to see it pretty much every time I was solving a problem with solc-js and I always ended up hacking it in with debug prints. Having this as an option would make working with solc-js much more convenient for me and I think it might be useful for others too.

@cameel cameel requested review from axic, leonardoalt and chriseth July 9, 2021 15:46
@cameel cameel self-assigned this Jul 9, 2021
Comment on lines +78 to +119
if (program.verbose)
console.log('>>> Retrying compilation with SMT:\n' + toFormattedJson(inputJSON) + "\n")
output = reformatJsonIfRequested(solc.compile(JSON.stringify(inputJSON), callbacks));
Copy link
Member Author

Choose a reason for hiding this comment

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

@leonardoalt This is a bit off-topic in this PR, but why do we have to retry here? Can't it be detected based on input alone?

Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

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

The goal and changes look ok to me (except the question to @leonardoalt), but I am a bit wary of adding too many features here, as that also means maintaining them.

I'd also suggest to ensure the options which are available match 100% with solc.

@cameel cameel force-pushed the solcjs-pretty-json-and-verbose-options branch from 9ecef3a to aa49a83 Compare September 22, 2021 15:30
@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.827% when pulling aa49a83 on solcjs-pretty-json-and-verbose-options into fc6671c on master.

@cameel
Copy link
Member Author

cameel commented Sep 22, 2021

These two are actually worth it in my opinion. Maintenance cost is minimal and they save time when debugging solc-js.

I'd also suggest to ensure the options which are available match 100% with solc.

They're unfortunately not but I think they're close enough.

  • --verbose was not needed so far in solc. At least in the sense used here - it does not construct JSON input but gets it as a parameter.
  • --pretty-json is present in solc but it has --json-indent which I did not add here to keep the interface minimal. Instead I just hard-coded set the default indent here to 4 spaces.

@cameel cameel merged commit 1ecce4b into master Sep 22, 2021
@cameel cameel deleted the solcjs-pretty-json-and-verbose-options branch September 22, 2021 15:36
@axic
Copy link
Member

axic commented Sep 22, 2021

--pretty-json.is present in solc but it has --json-indent which I did not add here to keep the interface minimal. Instead I just hard-coded set the default indent here to 4 spaces.

I guess as long as the behaviour is not different (just lacks features) it is fine.

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.

3 participants