Support coloured messages in standard json

[axic]

When we made the coloured output default (aka the new error message formatter), we did not enable it for standard json, only the CLI.

In the standard json we currently have all the components (message, errorCode, sourceLocation, ...) and single formattedMessage field which has an output matching the CLI.

We have a couple of options:

1. Enable colours by default and do not allow it to be turned off
2. Enable colours by default and provide an option to disable
3. Do not enable colours by default and provide an option to enable
4. Store the coloured message in a new field (such as coloredFormattedMessage

The option to enable/disable it would need to be part of the settings in the input.

[cameel]

We already have --color/--no-color CLI options, they just do not affect Standard JSON. Same for --pretty-json (which was quite unexpected to me at first). Setting the default for Standard JSON to --no-color and making these options affect both JSON and non-JSON mode should not break any tools so I think it would be the best choice.

By the way, if we start colorizing standard JSON then I think that --color/--no-color should affect --combined-json too for consistency.

[axic]

I'm not sure it is a good idea combining CLI options and standard json settings. --pretty-json is fine, but if you use --color to influence standard json output, then what do you do with the setting within standard json?

[cameel]

@axic Do we actually need a setting inside Standard JSON? Description does not state that outright but I thought it was about colorizing JSON when it's displayed in the terminal. I.e. with --standard-json but not necessarily solc-js. I think that a command-line flag makes sense in that usage.

But if it's supposed to work with solc-js too - do ANSI escape sequences embedded in strings inside JSON even make sense outside of the terminal? I see there are some JS libs for parsing them but if it's meant for JS tools, I think they would highly prefer some semantic markers that can be used to style them in a custom way rather than hard-coded colors.

[chriseth]

Can someone check how coloured strings would be displayed by e.g. remix or vs code?

[axic]

Do we actually need a setting inside Standard JSON? Description does not state that outright but I thought it was about colorizing JSON when it's displayed in the terminal. I.e. with --standard-json but not necessarily solc-js. I think that a command-line flag makes sense in that usage.

This is definitely not about colourising the JSON, but rather including colours in the formattedMessage. (That being said, anyone using | jq could also benefit.)

Can someone check how coloured strings would be displayed by e.g. remix or vs code?

There are libraries which can turn ANSI escapes into HTML markup. I think Remix had something like that for some purposes, but I'm not 100% sure, @yann300?

That is why this should be off by default, but tools can request it and process it.

I think they would highly prefer some semantic markers that can be used to style them in a custom way rather than hard-coded colors.

They already have that by now using formattedMessage, they can build up the contents themselves. The colour output in formattedMessage is for those who do not want to deal with formatting.

[cameel]

Looks like Hardhat wants to add error coloring so this could be useful to them: NomicFoundation/hardhat#680. I asked if ANSI escapes would work for them.

BTW, I have created a separate issue for --pretty-json: #11583. It's an easy one so it perfectly fits the good first issue label.

They already have that by now using formattedMessage, they can build up the contents themselves. The colour output in formattedMessage is for those who do not want to deal with formatting.

I just checked to see how it's formatted and expected to see some markup in there but looks like it just includes the error type and a code snippet with a marker pointing out the error:

"formattedMessage": "ParserError: Source \"util.sol\" not found: File outside of allowed directories.\n --> contract.sol:1:1:\n  |\n1 | import \"./util.sol\";\n  | ^^^^^^^^^^^^^^^^^^^^\n\n",
"message": "Source \"util.sol\" not found: File outside of allowed directories.",

I don't think it's that's easy to style unless you start actually parsing the messages.

[christianparpart]

Can we talk about this in the next meeting? I as i was doing the prior work, i have some questions (/concerns).

[christianparpart]

1. Enable colours by default and do not allow it to be turned off

Do you mean also for the existing case when printing to the CLI? I think that should still be guarded by isatty(STDOUT_FILENO) unless explicitly enable/disabled.

4. Store the coloured message in a new field (such as coloredFormattedMessage

If the IDEs are printing these messages as-is to the user, I think that does make sense - at first though I was a little scared/confused, because I may have thought that the IDE would not do that, because we provide source location offsets and the IDE could just highlight in the source code.

I think enabling that by default might not be good due to existing software not expecting this (?).
Stripping off VT (CSI) sequences however should be very easy (this FSM might scare on first sight, but we are only interested in the CSI case, that can be matched via basic regex).

Instead of providing the error message twice, I'd suggest to add a boolean "colored" flag that by default would be false (to stay backwards compatible).

WRT jq I think the desired option would be --raw-output.

[christianparpart]

I had a short call with @cameel, also to clarify my concerns and whether I am understanding the mission right here. I wanted to summarize now at least some thoughts we came up with in the call:

• underlines don't even have to be mimmicked via ^^^^ lines as VT does have native support for (even: curly underlines that can get a different color as the text they're being printed under, say yellow for warning and red for error. besides curly there is also support for single/double/dotted underline (exotic left out).
• we should document preciely what VT sequences we do produce: LF for newline, SGR reset (CSI m), 16-indexed color setting for foreground and (apprently?) also background as well as RGB color setting (at least CSI 48 ; r ; g ; b m). It would be simple to also add underline-mode and underline-color to it, because (surprise :-D) it's the exact same VT sequence, just with different numbers between CSI and m). CSI is the shorthand for "\033[".
• it should be mentioned that we currently indent via spaces, and that would require a monospaced font when displaying the messages (that one could probably be mitigated by using native underline sequences instead of manually writing ^^^'s under a source fragment.
• it at least came up as an idea: one could even syntax-highlight the printed AST fragment :)
• the original error message can remain the same but the additional source reference would require an additional JSON field in the reply (that may or may not be colored, based on what the client requested).

[cameel]

There was also a question of whether colored formattedMessage would be useful to tools. @christianparpart pointed out that it's only the actual console tools (like Hardhat or Truffle) that would potentially use it. GUI tools (like Remix or vscode-solidity) likely want only the error description to show it in a tooltip and don't need the underlining or the affected source code line. Since we do not really color anything inside the message itself, colored output would not benefit them much anyway.

[axic]

They already have that by now using formattedMessage, they can build up the contents themselves. The colour output in formattedMessage is for those who do not want to deal with formatting.

I just checked to see how it's formatted and expected to see some markup in there but looks like it just includes the error type and a code snippet with a marker pointing out the error:

"formattedMessage": "ParserError: Source "util.sol" not found: File outside of allowed directories.\n --> contract.sol:1:1:\n |\n1 | import "./util.sol";\n | ^^^^^^^^^^^^^^^^^^^^\n\n",
"message": "Source "util.sol" not found: File outside of allowed directories.",

@cameel It was a typo in my message, I meant that with message and all the other fields they can replicate what we provide under formattedMessage. formattedMessage is that same what we print on the CLI.

[axic]

Enable colours by default and do not allow it to be turned off
Do you mean also for the existing case when printing to the CLI?

@christianparpart this issue is solely about standard json, let's please leave the CLI out of the discussion 😅

[axic]

There was also a question of whether colored formattedMessage would be useful to tools. @christianparpart pointed out that it's only the actual console tools (like Hardhat or Truffle) that would potentially use it. GUI tools (like Remix or vscode-solidity) likely want only the error description to show it in a tooltip and don't need the underlining or the affected source code line. Since we do not really color anything inside the message itself, colored output would not benefit them much anyway.

GUI tools can use all the other fields we have in the message. If they want nice formatting and tooltips, they should just build it themselves and ignore formattedMessage. If somehow we do not export a given field, then we should fix that, as that was the initial goal of this structure.

[axic]

@cameel this tool for example can parse ANSI sequences and craft a group of spans for HTML: https://www.npmjs.com/package/ansicolor

[cameel]

I think I've seen this one (or something similar). I was just arguing that GUI tools might prefer a different markup but you are right that they can use other fields to construct the message. So I think ANSI sequences are fine if console tools would use them.

And I think they would. For example Truffle just prints all messages without any coloring and some people are annoyed enough to write custom scripts to colorize the output: Colorizing truffle compile output. This seems like a waste since compiler already has this info. I guess Truffle could construct colored message from components but for whatever reason it does not (seems like there was no feature request for that though so maybe it's just that nobody thought to ask? :)). I guess if we provided these colored messages in Standard JSON, Truffle would start using them right away because it would require very little effort.

[cameel]

I have submitted a feature request: trufflesuite/truffle#4305.

By the way, I just noticed that solc formats both warnings and errors in red. Is that intentional? I saw the initial mockup in #3046 and there warnings were proposed to be yellow. But then #5511 had them all red for some reason and that's how it went in.

[axic]

By the way, I just noticed that solc formats both warnings and errors in red. Is that intentional? I saw the initial mockup in #3046 and there warnings were proposed to be yellow. But then #5511 had them all red for some reason and that's how it went in.

Probably just a hiccup.

I have submitted a feature request: trufflesuite/truffle#4305.

Also clarified on NomicFoundation/hardhat#680 that hardhat is not used outside of consoles, so using ANSI is fine.

[axic]

A concrete proposal (option 3) from above): settings.ansiColors (bool). It is set to false by default (if omitted). If present and set to true, then fields in the output JSON can contain ANSI color sequences.

[axic]

@cameel I think it is not a good idea marking it good first issue before we agreed on the design. Let's try to do it on tomorrow's call.

[cameel]

Well, ok then, I'm taking it off.

The implementation is going to be straightforward whatever we choose though so I thought it was appropriate. And if anyone wants to try before them, we can always say that it's still waiting for design discussion.

[axic]

Yes, this is one of those unfortunate issues where discussing it takes 10x-100x the time it takes to implement it 🙈

[fvictorio]

I like when things work in a "core functionality that can be used by anyone" + "the tool builds upon that functionality as anyone else". I'll clarify what I mean.

The ideal thing, in my mind, would be for the standard json to work as a "source of truth", that is then used by the compiler CLI to generate its output in some opinionated way. That is, as much as possible, the compiler only relies on the JSON (or the equivalent internal data structure). One example would be:

• The JSON has a message, and a "level". The CLI decides that messages of level "warning" are printed as yellow and messages of level "error" are printed as red. This is the place where ANSI escape chars are inserted (to me, it doesn't make sense to have them as part of the JSON output).
• Anyone can consume the JSON and use it in a different way. For example, Remix can use this and render it in HTML in whatever way they want.

I understand that this is not possible right now, since the formattedMessage entry already has color characters. But IMO this is the right direction to move towards. So my personal preference would be to avoid adding the noColor option, to eventually deprecate the formattedMessage entry, and to provide any metadata necessary so that anyone can format the message however they want.

I know this is a very disruptive proposal, and I should have participated before. Sorry about that!

[cameel]

@fvictorio

I understand that this is not possible right now, since the formattedMessage entry already has color characters.

I don't think it does. It's formatted in a way suitable for the console (assumes fixed-width font and is just a single string with newlines) but currently it does not have any color info. Adding ANSI sequences to it what this issue proposes.

But IMO this is the right direction to move towards. So my personal preference would be to avoid adding the noColor option, to eventually deprecate the formattedMessage entry, and to provide any metadata necessary so that anyone can format the message however they want.

The metadata is already there. Here's an example what you get from Standard JSON now (not all fields are always present);

{
    "errors":
    [
        ...
        {
            "component": "general",
            "errorCode": "4334",
            "formattedMessage": "TypeError: Trying to override non-virtual function. Did you forget to add \"virtual\"?\n --> C.sol:2:5:\n  |\n2 |     function f() public {}\n  |     ^^^^^^^^^^^^^^^^^^^^^^\nNote: Overriding function is here:\n --> C.sol:6:5:\n  |\n6 |     function f() internal\n  |     ^ (Relevant source part starts here and spans across multiple lines).\n\n",
            "message": "Trying to override non-virtual function. Did you forget to add \"virtual\"?",
            "secondarySourceLocations":
            [
                {
                    "end": 102,
                    "file": "C.sol",
                    "message": "Overriding function is here:",
                    "start": 65
                }
            ],
            "severity": "error",
            "sourceLocation":
            {
                "end": 39,
                "file": "C.sol",
                "start": 17
            },
            "type": "TypeError"
        },
        ...
    ],
    "sources": {}
}

So what you're proposing is already possible. Everything needed to reconstruct the message is already there. The code snippets are not included but they can be easily extracted from source present in Standard JSON input based on the provided locations.

The goal of this issue, however, is to give tools an alternative to this. Currently most of them ignore these components and just use formattedMessage instead. Giving them an option to get ANSI colors in there would be a pragmatic move to let them display colored error messages with little effort. Deprecating formattedMessage would probably fix the situation too by forcing them to start formatting messages on their own but it might be taken as much more hostile :)

[axic]

Also note that we had these fields from the very beginning (and is part of the documentation), but no single tool decided to use any of those and just sticked to formattedMessage. I imagine because getting the source formatting nice is just extra work no one wants to do.

[fvictorio]

Oh, sorry, I completely misunderstood the issue. I see what you mean now! I'll think about this a little more.

[fvictorio]

This is a hard question. If I'm understanding correctly, there are three options here:

• Add ANSI colors to the formattedMessage entry, and add a noColor option to the JSON input.
• Add ANSI colors but without an option in the input. That is, assume that formattedMessage will always be shown in a terminal.
• Don't do anything. If someone wants colors, they'll have to either use the same color for the full formatted message (we are doing that in Hardhat) or build a custom message with the other parts of the error object.

I don't know which option is better. I think that if ANSI colors are added, we might just disable them because we save the JSON input/output to disk, and having colors there feels wrong. (I'm not 100% sure though, but this is my first guess)

Also note that we had these fields from the very beginning (and is part of the documentation), but no single tool decided to use any of those and just sticked to formattedMessage. I imagine because getting the source formatting nice is just extra work no one wants to do.

To be honest, I didn't know about this 😬 And now I kind of want to generate our own formatted error messages. But at the same time, yes, it is extra work and I have no idea when we'll have the bandwidth to do it.

[cameel]

I think that if ANSI colors are added, we might just disable them because we save the JSON input/output to disk, and having colors there feels wrong.

That's a good point against an option that modifies formattedMessage. Doing it this way does make the JSON output less reusable. So if we go forward with it, I think a new field would be better because it can be ignored and keeps formattedMessage available, even if you get stored JSON output from elsewhere.

[fvictorio]

@alcuadrado mentioned that having a new ansiColoredMessage (or however it's called) has a couple of advantages.

First, this would avoid the need of having a new flag in the input, which is problematic for verification because it affects the metadata (it would suck if your verification fails because you had a different colors config when the contract was deployed). Second, having a different field means that it's easier to support all versions of solc; otherwise, we should know that after some version the formattedMessage field has different assumptions.

If we have this field, then the part of the code that handles showing the messages can have a simple logic that works for all versions:

if (error.ansiColoredMessage) {
  console.log(error.ansiColoredMessage)
} else { 
   console.log(colorize(error.formattedMessage)
}

[axic]

This is a hard question. If I'm understanding correctly, there are three options here:

1. Add ANSI colors to the formattedMessage entry, and add a noColor option to the JSON input.
2. Add ANSI colors but without an option in the input. That is, assume that formattedMessage will always be shown in a terminal.
3. Don't do anything. If someone wants colors, they'll have to either use the same color for the full formatted message (we are doing that in Hardhat) or build a custom message with the other parts of the error object.

The main options according to this:

1. Add ANSI colours to formattedMessage iff the settings.anisColors is true. Otherwise (the default) stay as today.
2. Do nothing.
3. Introduce more complex version of formattedMessage which has spans.

[d-xo]

In dapp we currently just dump the output from formattedMessage and don't apply any colorization (although I did just open a pr to differentiate between warning and error messages based on the script you linked above @cameel 🙏).

I would be happy to see full colorized output included in the output json, and would definitely make use of it in dapp, especially if it included e.g. syntax highlighting for the solidity snippets in the output.

I don't have a strong preference regarding interface, either a switch in the input settings, or a new field in the output are fine by me, but as @fvictorio mentions a new field will probably make for a pretty clean impementation.

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[github-actions]

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.