Skip to content

Add numeric comparison filters - #147

Open
droyad wants to merge 2 commits into
masterfrom
robw/issue-114-comparison-operators
Open

droyad wants to merge 2 commits into
masterfrom
robw/issue-114-comparison-operators

Conversation

@droyad

@droyad droyad commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Adds LessThan, LessThanOrEqual, GreaterThan and GreaterThanOrEqual filters, so a numeric comparison can be used anywhere a variable expression can be — including as a run condition on its own.

#{Octopus.Action[Get Days Until Expiration].Output.DaysUntilExpiration | LessThan 31}

That replaces the workaround in the issue, which was to #{calc {X} - 31} into a variable and then test it with | StartsWith "-".

Why filters rather than new syntax

The issue proposed three syntaxes with no stated preference: bare #{X < 31}, a | LessThan 31 filter, or #{calc {X} < 31}. The filter form wins on more than just cost:

#{if <expression>} already accepts a full filter chain, so a single filter covers run conditions, conditionals and #{elseif} chains at once:

#{if DaysUntilExpiration | LessThan 31}Expiring soon#{/if}
#{if Count | LessThan 10}low#{elseif Count | LessThan 100}medium#{else}high#{/if}

It also means no grammar change at all — TemplateParser.cs is untouched, so there is no way for this to alter how any existing template containing < or > is parsed. Given that Octostache parses every variable in every Octopus deployment, that seemed worth a lot. It composes with other filters (#{Total | Trim | GreaterThan 100}) for free, and leaves the door open to adding the bare #{X < 31} form later as sugar over the same logic.

#{calc {X} < 31} was rejected on design grounds: calc's entire type model is double?, and a comparison yields a boolean.

Semantics

Each decision follows an existing precedent in the codebase:

Case Result Precedent
Both sides numeric "true" / "false" StartsWith, EndsWith, Contains, Match
Either side non-numeric null → token left unevaluated, recorded as missing, not truthy CalculationVariable.Evaluate
Not exactly one argument null StartsWith with 0 or 2+ options

Returning "true"/"false" keeps these consistent with the existing text-comparison filters and with IsTruthy, so they behave correctly as run conditions.

Number parsing is invariant-culture. double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture) — a template gives the same answer regardless of the culture of the machine evaluating it, rather than a run condition that can flip depending on which worker picks it up. NumberStyles.Float deliberately excludes thousands separators, so an ambiguous "1,000" is treated as non-numeric instead of being silently read as 1.

Note this is deliberately not consistent with #{calc}, which parses and formats using the current culture in four places. Nothing in the Octostache readme or the public docs specifies a number format for calc, so there was no documented contract to match, and I did not want to propagate machine-dependent behaviour into something new. calc is left alone in this PR.

NaN and Infinity parse and then follow ordinary double comparison semantics (so NaN | LessThan 31 and NaN | GreaterThanOrEqual 31 are both false). Pinned by a test rather than left to chance.

Out of scope

  • AND/OR operators, requested in a comment on the issue. Not blocked by this — they would be additive, as either filters or grammar.
  • The bare #{X < 31} and #{calc {X} < 31} forms.
  • calc's culture handling.

Testing

TDD: tests written first and watched fail, then the implementation. Full suite is green on both target frameworks — 696 passed, 0 failed on net10.0 and net48 — which is the backwards-compatibility safety net for a library this widely exercised.

Coverage includes each operator's boundary behaviour (5 | LessThan 5 false vs 5 | LessThanOrEqual 5 true), numeric-not-lexicographic ordering (9 | LessThan 10), negatives, decimals, exponents, surrounding whitespace, non-numeric on either side, wrong arity, variable options, filter chaining, case-insensitive filter names, and the invariant-culture decision pinned via "1,5" and "1,000". Integration tests cover the issue's run condition verbatim (indexed output variable and all), a missing/misspelled variable evaluating to false rather than silently passing, and comparisons inside #{if} and #{elseif} chains.

Docs

README.md gains a short "Comparison filters" section. The public docs at octopus.com/docs/projects/variables/variable-filters will need a matching entry under Comparison filters — separate repo, not done here.

Resolves #114

🤖 Generated with Claude Code

@benPearce1 benPearce1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but needs some extra thought might be the error states

Comment on lines +22 to +23
if (argument == null || options.Length != 1)
return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably return a formatted error

return null;

if (!TryParseNumber(argument, out var left) || !TryParseNumber(options[0], out var right))
return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it should just return false, or an error

droyad and others added 2 commits September 18, 2026 14:56
Adds LessThan, LessThanOrEqual, GreaterThan and GreaterThanOrEqual, so a
numeric comparison can be used anywhere a variable expression can be -
including as a run condition on its own, which is what #114 asked for.

Implemented as filters rather than as new grammar. `#{if <expression>}`
already accepts a full filter chain, so this covers run conditions,
conditionals and `#{elseif}` chains without touching the parser, and
without changing how any existing template is parsed.

Both sides are parsed with the invariant culture so that a template gives
the same answer wherever it is evaluated. NumberStyles.Float excludes
thousands separators, so an ambiguous value like "1,000" is treated as
non-numeric rather than being read as 1. A value that is not numeric
returns null, which leaves the token unevaluated and is not truthy -
matching the existing text comparison filters.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses review feedback on the error states.

A value that is present but not a number now compares as false, rather
than leaving the expression unevaluated: no number can satisfy the
comparison, so false is the honest answer.

The wrong number of arguments is a mistake in the template itself that
no value could make valid, so it now returns Error.Format, consistent
with UriPart. The caller is captured with CallerMemberName so the
message names the filter the author wrote rather than the private helper.

An unresolved variable still returns null and is reported as a missing
token, consistent with every other filter. Returning false there would
make a misspelled variable name quietly compare as false instead of
being reported.

Keeping false (rather than a formatted error) for a non-numeric value
matters because Error.Format strings are truthy - IsTruthy only treats
"", "0", "no" and "false" as falsy - so returning an error from a
comparison would make a broken run condition evaluate true and run the
step. All three failure modes are non-truthy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@droyad
droyad force-pushed the robw/issue-114-comparison-operators branch from 9e55c4f to af08732 Compare September 18, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Support for comparison operators (<, >, <=, >=)

2 participants