fix: case-insensitive boolean comparison in ConditionalEvaluator#73
Conversation
AreEqual used case-sensitive ToString() comparison, causing expressions like `IsActive = true` to fail because C#'s true.ToString() returns "True" (capital T) while the literal token is "true" (lowercase). Use OrdinalIgnoreCase comparison. Add 6 unit tests for explicit boolean comparisons and document `= true` / `= false` syntax in docs and README.
There was a problem hiding this comment.
Pull request overview
Fixes a bug in Templify’s conditional evaluation where explicit boolean comparisons (e.g., IsActive = true) could fail due to case-sensitive string comparison of boolean values.
Changes:
- Update
ConditionalEvaluator.AreEqualto use case-insensitive string comparison. - Add unit tests covering explicit boolean comparisons (including nested paths).
- Update documentation and README examples to include
= true/= falsecomparisons.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/for-developers/condition-evaluation.md | Adds examples for explicit boolean comparisons in standalone condition evaluation docs. |
| TriasDev.Templify/Conditionals/ConditionalEvaluator.cs | Changes equality comparison behavior used by conditional operators. |
| TriasDev.Templify.Tests/ConditionalEvaluatorTests.cs | Adds unit tests validating explicit boolean equality evaluation (true/false, nested paths). |
| README.md | Updates usage example to demonstrate IsActive = true in batch evaluation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| return left.ToString() == right.ToString(); | ||
| return string.Equals(left.ToString(), right.ToString(), StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
Changing AreEqual to use OrdinalIgnoreCase makes all equality comparisons case-insensitive, not just boolean ones. This is a behavioral change for string comparisons (e.g., Status = Active will now match "active"), which contradicts the PR description’s claim that existing equality comparisons are unaffected and could break templates that rely on case-sensitive matching. Consider narrowing the fix to boolean comparisons only (e.g., when either side is a bool or a bool-like string, parse/compare as bool), and otherwise keep the prior case-sensitive semantics for general string equality.
Address review feedback: only use case-insensitive comparison when
either side is a bool or a boolean literal ("true"/"false"). String
comparisons like Status = "Active" remain case-sensitive.
Add test asserting string comparison stays case-sensitive.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #72
Summary
ConditionalEvaluator.AreEqualused case-sensitiveToString()comparison, causingIsActive = true/IsActive = falseexpressions to silently fail (C#'strue.ToString()returns"True", not"true")boolor a boolean literal ("true"/"false"). String comparisons remain case-sensitive.=,!=, nested paths, case-sensitive string guard)= true/= falseexamples tocondition-evaluation.mdandREADME.mdTest plan
dotnet format --verify-no-changescleanStatus = "Active") remain case-sensitive (guarded by test)!=operator works correctly with boolean literals