command: Better visual hierarchy for diagnostics #27343
Conversation
apparentlymart
force-pushed
the
f-diagnostic-lines
branch
from
1432d2d to
64b4669
Compare
Codecov Report
|
|
I like this! Would it be possible to use the DOS box drawing characters for Windows builds? |
|
That's an interesting idea, @alisdair ... Dealing with the layering of sending output through Go-string-oriented APIs in One constraint here is that in Unicode I used (This doesn't render quite right in my browser, it turns out, but I expect it would look better in the real Windows Console where the character cells have no extra spacing between them. Maybe it looks better in other browsers or in browsers on other platforms. 🤷♂️ ) You also make me realize that, legacy ANSI codepage weirdness aside, it would still be possible to conditionally emit different characters based on target OS and thus print out UTF-8-encoded Unicode box drawing characters on every platform other than Windows, at the expense of having another annoying platform-specific codepath. |
apparentlymart
force-pushed
the
f-diagnostic-lines
branch
from
64b4669 to
da0e105
Compare
|
This PR sent me on a little yak-shaving adventure: I booted up my Windows 10 VM with the intent to try out some of the options we were discussing upthread, such as emitting cp437-encoded box drawing characters instead of ASCII art or UTF-8. However, when I got there I found that an unrelated earlier commit had inadvertently removed our Windows Console API compatibility shims as a side-effect of other work, and so we ended up having to decide whether to spend time figuring out how to repair that or to move forward with our already-planned #22487 a little early and implement Windows 10-compatible virtual terminal setup instead. After some further research we decided to move ahead with removing official support for Windows 8 and for the early Windows 10 minor releases that didn't have the Unicode console buffer and virtual terminal processing support yet, following recommendations from Microsoft. (You can read more about that over in #22487) The upshot of all of this is that I ended up having an unexpected half-week diversion into implementing Windows-compatible terminal initialization so that we can now use both virtual terminal sequences and UTF-8 in the Windows Console. Today I was finally able to get back to working on this PR, and thankfully I was able to just quickly revert back to my original implementation with the unicode box drawing characters thanks to our new requirement for a UTF-8 capable terminal on all platforms. The latest version of the commit on this PR therefore now implements what I showed in my first screenshot/snippet in the opening comment, rather than the second. What do you think about this now, @alisdair? |
|
I think it's great, and reads much more clearly than the ASCII approach! Works great with cmd.exe and Powershell:
|
apparentlymart
force-pushed
the
f-diagnostic-lines
branch
from
da0e105 to
fd259fd
Compare
I frequently see people attempting to ask questions about Terraform's error and warning messages but either only copying part of the message or accidentally copying a surrounding paragraph that isn't part of the message. While I'm sure some of these are just "careless" mistakes, I've also noticed that this has sometimes overlapped with someone asking a question whose answer is written directly in the part of the message they didn't include when copying, and so I have a theory that our current output doesn't create a good enough visual hierarchy for sighted users to understand where the diagnostic messages start and end when we show them in close proximity to other content, or to other diagnostic messages. As a result, some folks fail to notice the relevant message that might've answered their question. I tried a few different experiments for different approaches here, such as adding more horizontal rules to the output and coloring the detail text differently, but the approach that felt like the nicest compromise to me was what's implemented here, which is to add a vertical line along the left edge of each diagnostic message, colored to match with the typical color we use for each diagnostic severity. This means that the diagnostics end up slightly indented from what's around them, and the vertical line seems to help subtly signal how we intended the content to be grouped together.
apparentlymart
force-pushed
the
f-diagnostic-lines
branch
from
fd259fd to
ff9ecec
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked as resolved and limited conversation to collaborators
I frequently see people attempting to ask questions (e.g. in the community forum) about Terraform's error and warning messages but either only copying part of the message or accidentally copying a surrounding paragraph that isn't part of the message.
While I'm sure some of these are just "careless" mistakes, I've also noticed that this has sometimes overlapped with someone asking a question whose answer is written directly in the part of the message they didn't include when copying, and so I have a theory that our current output doesn't create a good enough visual hierarchy for sighted users to understand where the diagnostic messages start and end when we show them in close proximity to other content, or to other diagnostic messages. As a result, some folks fail to notice the relevant paragraph that might've answered their question.
I tried a few different experiments for different approaches here, such as adding more horizontal rules to the output and coloring the detail text differently, but the approach that felt like the nicest compromise to me was what's implemented here, which is to add a vertical line along the left edge of each diagnostic message, colored to match with the typical color we use for the diagnostic's severity. This means that the diagnostics end up slightly indented from what's around them, and the vertical line seems to help subtly signal how we intended the content to be grouped together, particularly when multiple diagnostics appear together.
I originally tried this with Unicode box drawing characters, which gave a nice result:
However, we're currently still constrained by supporting Windows 8's legacy Windows Console, which has no Unicode support and so cannot handle these box drawing characters.
As a compromise for now I've switched them out for similar ASCII-art-style characters for what I've submitted here, similar to how we've typically created horizontal rules in other commands today:
This ASCII-art version is not ideal because it's ambiguous in non-visual terminals and it's not as elegant looking (subjectively), but I think overall this is a good tradeoff for the current constraints and that non-visual terminals will encounter similar sorts of output from other software and thus should have heuristics to handle it somewhat gracefully. Hopefully it won't be long before we can officially end our Windows 8 support (#22487) and thus require a modern terminal which supports Unicode properly, and then we can swap back to the box drawing characters, which I've saved in a source code comment here so we can be reminded about it. 😀
Unfortunately, this further aggravates the problem that we have various tests that try to recognize errors by matching substrings of the error output, which only works as long as those substrings happen to wrap such that they aren't broken by a newline. They can now also end up broken by an intrusive colored
|character. In order to avoid embarking on the inevitable eventual error-testing-refactoring project we'll have to do someday, I've just updated the tests to expect the new output shape here. That's far from ideal, but not actually a new problem.