OctopusDeploy / Octostache

| Public | The variable substitution syntax for Octopus Deploy
Other
5 stars 24 forks source link

Adds ability to reverse iteration order #79

Closed mjhilton closed 2 years ago

mjhilton commented 2 years ago

Changes

Adds a new "reversed" keyword to the end of the #{each} construct, so that users can write templates like:

#{each note in Package.ReleaseNotes reversed}
  * #{note}
#{/each}

The template will reverse the contents of the collection before iterating.

Linked Issues

ghost commented 2 years ago

It looks to me that introducing a new keyword would potentially be a breaking change. For example, if someone has a template using a list called reversed (not an uncommon name), then parsing will fail since it is interpreted as the keyword rather than the variable name:

[Fact]
public void TestListNamedReversed()
{
    var variables = new Dictionary<string, string>
    {
        { "reversed[0]", "a" },
        { "reversed[1]", "b" },
        { "reversed[2]", "c" },
    };

    var result = Evaluate("#{each item in reversed}#{item}#{/each}", variables);
    result.Should().Be("abc");   
    // actual: "#{each item in reversed}#{item}#{/each}" - failed to parse string as template
}
mjhilton commented 2 years ago

It looks to me that introducing a new keyword would potentially be a breaking change

Great catch, thankyou @YihaoOct. I wonder if we could implement a new filter, instead of changing the syntax of the #{each} template. Something along the lines of

#{each item in list | Reversed} or #{each item in list | Sort Descending}

ghost commented 2 years ago

Yep, I was thinking of the same approach as well. If we can enable function calls for lists, that would be awesome.

The challenges I can foresee now are 1) function calls are currently only supported on texts (strings), we need more fundamental changes to the parser, token definition and evaluator to enable this on iterations; and 2) the thing that can be used in the #{each} syntax can be either a list (unindexed) or a dictionary (indexed), and it will require some consideration to make sure the functions work for both indexed and unindexed collections, with a reasonable behaviour (e.g. what does myDictionary | SortDescending mean? Sort by keys or values? Or should it just fail validation?).

That said, if we just aim to make list | Reversed working as a temporary special syntax, rather than enable the full function call syntax, it should be relatively easy.

Note: there's also a perhaps related issue requesting function calls to be enabled in #{if} syntax: https://github.com/OctopusDeploy/Octostache/issues/65

mjhilton commented 2 years ago

Superseded by #83