fix(validate): stop render blocking on unresolved variables validate allows - #196
Merged
Merged
Conversation
…allows
The two commands disagreed on the same file:
$ rustmotion validate -f var.json
Valid scenario: 1 scene(s) in 1 view(s) exit 0
$ rustmotion render -f var.json -o var.mp4
Error: Validation failed: 1 unresolved variable(s).
Run `rustmotion validate -f <file>` to see details. exit 1
The refusal pointed at a command that reports the scenario as valid, so there
was no way to see the problem.
validate was the one following the documented decision. variables.rs (constat
#7) settled that a leftover $word cannot be told apart from legitimate literal
$ content — a price tag, a terminal $PATH — and is therefore a loud warning,
not a rejection; a hard failure would break any scenario with a $ in its text.
is_blocking never got the memo.
Drop the unresolved_vars term from is_blocking. It stays in is_clean, so the
warning is still printed by both commands — the change is what happens next,
not whether it is reported.
Nothing that reaches this point is a diagnosable typo: every declared variable
is present in defaults ∪ overrides, so substitute cannot leave one behind, and
a for-each/use mistake is already named and located by expand.rs.
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.
Closes #187.
What
The refusal told the user to run
validate, which reports the file as valid. There was no command that showed the problem.Which side was wrong
Not the one I expected.
validatewas following the documented decision —variables.rs(constat #7) settled that a leftover$wordcannot be told apart from legitimate literal$content (a price tag, a terminal$PATH, a shell$HOME), so it is reported as a loud warning rather than aborting; a hard rejection would break any existing scenario with a$in its text, and would do so only once the file happened to gain aconfigblock.ValidationReport::is_blockingnever got that memo, andrendergoes through it.So the fix is to remove the
unresolved_varsterm fromis_blocking, not to add it tovalidate.Behaviour change
A typo'd
$variablenow renders as literal text instead of stopping the render. That is the intended trade-off, and nothing that reaches this point is a diagnosable typo: every declared variable is present in defaults ∪ overrides, sosubstitutecannot leave one behind, and afor-each/usemistake is already named and located byexpand.rs.unresolved_varsstays inis_clean, so both commands still print the warning — what changed is what happens next, not whether it is reported.After
Test
an_unresolved_variable_warns_without_blockinguses"$9.99 a month"— the exact content the design decision is about — and asserts all three properties: it is reported, it does not block, andis_cleanstays false so it cannot be silently dropped.cargo test --workspacegreen,cargo fmt --checkandcargo clippy --all-targets -- -D warningsclean.