Point configuration errors at the exact line and make colour optional - #103
Merged
Conversation
Modelled on CORE's manifest error rendering: a located, colourised snippet that marks the line to change. Option errors marked the first line of the enclosing test or step, so a typo in `evaluatte:` on line 11 pointed at line 6. Option key positions are now recorded alongside the block positions already captured, and an error about an option marks that option's own key. The Key field on ConfigError, unused until now, carries which one. YAML syntax errors got no snippet at all — just `yaml: line 2: ...` with the position buried in the message. Any error carrying a YAML position is now converted to a located ConfigError on the way out of parsing, so a malformed suite gets the same treatment a semantic error gets. The conversion is skipped once load_from has inlined other files, since the line numbers no longer correspond to the file on disk. Colour is now controllable with --color: auto (the default, which colours only when stdout is a terminal and honours NO_COLOR), always, or never. It is resolved before anything can print, so an error rendered on the way out of parsing already honours the setting.
The unknown-option case pointed at the option's own key, but a value error such as timeout: -5 still marked the first line of the enclosing test or step. Both now take the option name from the front of the message — the convention every option error follows — and use that option's recorded position when the block really has an option by that name, falling back to the block otherwise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Modelled on CORE's manifest error rendering (
grewelltech/core,mvpbranch) — a located, colourised snippet that marks the line to change.Option errors pointed at the wrong line
A typo in
evaluatte:on line 11 marked line 6, the start of the test:DART already captured positions for each node/test/step block and for their
type:/node:keys, but not for individual option keys. It now records those too, so the error marks the key itself:ConfigError.Key— declared but unused until now — carries which option is at fault.Syntax errors got no snippet at all
The position was in the message text, and nothing rendered it. Any error carrying a YAML position is now converted to a located
ConfigErroron the way out of parsing:The conversion happens in one deferred choke point rather than at each
yaml.Unmarshal— the first parse is insidesubstituteVars, so a per-call-site fix would have missed it. It is skipped once!!load_fromhas inlined other files, because the line numbers no longer match the file on disk and a snippet pointing at the wrong line is worse than none.Colour is now optional
--color auto|always|never, defaulting toauto: colours when stdout is a terminal and honoursNO_COLOR(the cross-tool convention).alwaysforces it on for a pipe that renders ANSI;neverturns it off.It is resolved before anything can print, so an error rendered on the way out of parsing already honours the setting. An invalid value is rejected rather than silently treated as
auto.Verified:
--color alwaysemits escapes through a pipe,--color neverandNO_COLOR=1produce none.What I did not take from CORE
CORE also separates a short
Summaryfrom a longerDetailand renders a column caret. DART's messages are already single-sentence and specific, and itsSourceLocation.Columnis populated but points at the YAML key rather than the offending token within it, so a caret would often be misleading. Left alone rather than added speculatively — happy to revisit if you want the two-part shape.Verification
New tests cover position recovery from both YAML error shapes, that unlocated errors pass through untouched rather than acquiring a misleading location, that an already-located error keeps its own, and that an option key's recorded line differs from its block's. All example suites still pass
--check; build, vet, gofmt, and the full suite are green; the docs site builds.