CommandLineInterface.cpp is just one single huge mess right now :-).
Related to https://github.com/ethereum/solidity/issues/9579 and https://github.com/ethereum/solidity/issues/8722 and https://github.com/ethereum/solidity/issues/5306
Strange there wasn't an issue for this, it has been one goal for me at least :+1:
Of course there was :) #7813.
I'm closing it in favor of this one here. As short as its description is, it's still more detailed than in the old one :)
IMHO another argument for this: as far as I'm concerned https://github.com/ethereum/solidity/pull/9954 should not have been done like that, but rather the command line interface be fixed.
@ekpyron we should do both.
Need to also keep #8847 in mind, such as:
Some options available to the standard-json format do not seem to be available as CLI flags. I'm missing in particular source mappings for yul which was introduced only for the json options in #8378. It would be nice if feature parity between CLI flags and json opts was pursued as a general rule.
From https://github.com/ethereum/solidity/pull/10241#pullrequestreview-526581636: I think there should be just one top-level try...catch block around everything in the CLI interface that catches exceptions not properly handled below, reports any diagnostic info it finds and exits. Currently there are multiple ad-hoc try...catch with very limited scope that look as if they were going to handle errors but all they don't actually do anything, they just report and exit and that should be delegated to the top level.
I think we should really do #5303 before this and get full test coverage of all existing options. Otherwise we'll be dealing with random bugs that nobody noticed for months to come.
I have actually tried to start this recently by adding a boost test suite for CommandLineInterface class but it requires either making a lot of stuff public or splitting it into smaller components that can be tested independently.
Actually, I wonder how attached people are to the current command line options...
Ideally I would suggest to drop all of them and replace them by an extremely simple 1:1 translation from StandardJSON to new command line options in a breaking release - at least if we can come up with a nice way to do so. That way unit tests for StandardCompiler are basically enough to cover both CLI and standard json and only a few "proper" command line and standard json tests on top of that would be enough - and we never have to actually bother with https://github.com/ethereum/solidity/issues/5303 too much at all.
If we're afraid to break tooling too much like that, I'd write a thin wrapper script that covers the most commonly used old command line options and translates them to the new options, but with a "use-at-your-own-peril" plus "better-update-your-stuff" policy...
But yeah, I'm not sure we'll be able to agree on this and if we want to consider it, we'd probably need a draft of how the simple CLI to StandardJSON translation would look like in practice to decide if it is likely to increase the usage learning curve too much or not.
Actually, I wonder how attached people are to the current command line options...
See #10278. Answer is: pretty much :wink:
Also check https://github.com/ethereum/solidity/projects/44 for other proposed changes.
If I read that the main attachment is towards non-standard-json output options of the CLI. Actually I agree that we should keep those... or put differently: I would keep --standard-json as CLI option anyways, meaning proper json input and output - but if the compiler is instead invoked using the "simple translation of CLI options to standard-json" I also wouldn't output standard-json, but rather just plainly output the requested output components. And we could keep --combined-json to pack the output like that instead... but yeah, granted: things get a bit fuzzy there, I hadn't thought this through entirely...
Most helpful comment
Strange there wasn't an issue for this, it has been one goal for me at least :+1: