Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds ability to reverse iteration order #79

Closed
wants to merge 1 commit into from

Conversation

mjhilton
Copy link
Contributor

@mjhilton mjhilton commented Jun 2, 2022

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
Copy link

ghost commented Jun 3, 2022

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
Copy link
Contributor Author

mjhilton commented Jun 5, 2022

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
Copy link

ghost commented Jun 7, 2022

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: #65

@mjhilton
Copy link
Contributor Author

Superseded by #83

@mjhilton mjhilton closed this Jul 18, 2022
@mjhilton mjhilton deleted the matth/iteration-order branch July 18, 2022 07:32
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.

Possibility to support sorting for collections
1 participant