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.
Rather than using a prior value of null to indicate create, which is
imprecise because null is a valid output value, only plan values that
didn't exist in the prior state as Create changes.
The planning logic here was inspired by resources, but unlike resources
a null value is valid for an output and does not necessarily indicate it
is being removed. If both before and after are null, the change should
be NoOp. When an output is removed from the configuration, we have a
separate node to create a Delete action to remove it from the state.
This task relied on using `wget` to spider the entire site, which is no longer a
useful way of checking for broken links on a non-production instance of
terraform.io. Also, it didn't include a step for pausing until the server came
online, so the task wouldn't have functioned properly anyway.
In some terminal emulators, writing a character into the last column on a
row causes the terminal to immediately wrap to the beginning of the next
line, even if the very next character in the stream is a hard newline.
That can then lead to errant blank lines in the final output which make
it harder to navigate the visual hierarchy.
As a compromise to avoid this, we'll format our horizontal rules and
paragraphs to one column less than the terminal width. That does mean that
our horizontal rules won't _quite_ cover the whole terminal width, but
it seems like a good compromise in order to get consistent behavior across
a wider variety of terminal implementations.
We were previously using some ASCII art to create some visual divisions
between parts of the diagnostic output. Now that we are requiring a UTF-8
terminal we can print out box drawing characters instead.
We now require the output to accept UTF-8 and we can determine how wide
the terminal (if any) is, so here we begin to make use of that for the
"terraform plan" command.
The horizontal rule is now made of box drawing characters instead of
hyphens and fills the whole terminal width.
The paragraphs of text in the output are now also wrapped to fill the
terminal width, instead of the hard-wrapping we did before.
This is just a start down the road of making better use of the terminal
capabilities. Lots of other commands could benefit from updates like these
too.
Here we propagate in the initialized terminal.Streams from package main,
and then onwards to backends running in CLI mode.
This also replaces our use of helper/wrappedstreams to determine whether
stdin is a terminal or a pipe. helper/wrappedstreams returns incorrect
file descriptors on Windows, causing StdinPiped to always return false on
that platform and thus causing one of the odd behaviors discussed in
Finally, this includes some wrappers around the ability to look up the
number of columns in the terminal in preparation for use elsewhere. These
wrappers deal with the fact that our unit tests typically won't populate
meta.Streams.
We need to call into terminal.Init in early startup to make sure that we
either have a suitable Terminal or that we disable attempts to use virtual
terminal escape sequences.
This commit gets the terminal initialized but doesn't do much with it
after that. Subsequent commits will make more use of this.
This is a helper package that creates a very thin abstraction over
terminal setup, with the main goal being to deal with all of the extra
setup we need to do in order to get a UTF-8-supporting virtual terminal
on a Windows system.
This adds a CI job for running the new PR link checker for documentation.
[terraform-website PR 1574](https://github.com/hashicorp/terraform-website/pull/1574)
added a new link checking CI job specifically for warning about broken links in
pull requests. This link checker is optimized for:
- Running (relatively) quickly.
- Only reporting on files that were changed in the current PR, to avoid spamming
you with problems you had nothing to do with.
- Being transparent and simple to maintain. (Note that this is in conflict with
minimizing false positives/negatives! We try to give very few of both, but
completely eliminating them would result in an unaffordable maintenance
burden. We expect that some PRs will be merged with this job red.)
The tool is somewhat specific to our Middleman site builder, and we expect it
will be replaced or obviated in the transition to the Next.js platform... but in
the meantime, it should help make documentation slightly easier to maintain.
Remove any calls to testDiffFn and testApplyFn which don't effect the
test result. This way we have more tests using predictable provider
behavior, which is more likely to uncover legitimate regressions as the
particular behavior of the legacy mock provider logic does not need to
be taken into account.
Now that we don't need to track separate error values for the apply
logic, it's easier to reorganize the diagnostic handling to use a single
set of diagnostics.
Trying to track these error values as they wint into and out of the
instance apply methods was quite difficult. They were mis-assigned, and
in many cases lost diagnostic information.
Various pieces of the state and/or warnings were dropped when the
provider returns an error. Do a little cleanup of `.apply` to make the
logic easier to follow.
After verifying the remote backend workspace version is compatible with
the local Terraform version, we need to record that this check was
successful. Otherwise the fallback error handling path will run a strict
version check, even if the versions are compatible, which will cause an
unexpected failure.
Add reasonable default behavior to the mock provider, so that may do not
need to depend on the idiosyncrasies of the old (though updated) test
testDiffFn and to a lesser extend the testApplyFn. This behavior is
based directly upon the documented resource lifecycle, rather
than be an ad-hoc collection of behaviors from old tests.
As a proof of concept, remove all uses of testDiffFn from the plan
context tests that don't cause the tests to fail.
Nick Fagerlund