Added delimit & sort functions, tests and docs

[derekperkins]

Added a new function, delimit, that takes a slice, array or map and returns a string with all the values delimited by the specified string.

[halostatue]

Couldn’t that be:

func Delimit(seq interface{}, delimiter, last... string) (string, error) {

My thought there is that delimiter should always be a string, and it should be possible with last... to support a “last delimiter” that would fix the “and” problem.

[bep]

The optional "last" would support a specific (and popular) style guide in English, and would be useless to my Norwegian (I would like "første, andre og tredje (first, second and third)). So if you could come up with a "optional last delimiter" syntax it would be perfecto.

[halostatue]

In my mind, it would be used like:

{{ delimit seq ", " ", and " }}

[bep]

Yes, that would work.

[derekperkins]

It would still work for you the way it's described.

{{ delimit ["første", "andre", "tredje"]  ", " " og " }} => "første, andre og tredje" 
{{ delimit ["first", "second", "third"]  ", " " and " }} => "first, second and third" 
{{ delimit ["first", "second", "third"]  ", " ", and " }} => "first, second, and third" 

[derekperkins]

Wow, this got popular all of a sudden. :)

[derekperkins]

@bjornerik Should the keys be sorted or the values be sorted?

[derekperkins]

I just pushed the tested changes for adding a last parameter. I'm not 100% sold on the sorting idea. I like that it makes it testable, but other than that, sorting by keys or values just doesn't seem to add value.

[halostatue]

When dealing with a map, sorting by keys is unfortunate, but otherwise you will not get a stable rendering order. That is, if you have:

{
  "a": "apple",
  "b": "bacon",
  "c": "chocolate",
}

you will have a roughly random ordering each time:

1. apple, banana, chocolate
2. banana, apple, chocolate
3. chocolate, banana, apple
4. …

It only affects maps (and interfaces?), not sequences.

[derekperkins]

I get that the order isn't guaranteed, but that's kind of my point. When your data is coming into the delimit function, you've already lost any type of manual ordering, so whether the output is random or sorted, it doesn't seem like it'd matter to you. I guess the only use case I can think of is if you're outputting taxonomies and want the terms in alphabetical order... That would mean sorting by values and not keys for it to make sense on the front end.

[halostatue]

Ah. I see what you’re saying. I’d say leave it unsorted, and we can have a separate call to sort a sequence-able object, resulting in {{ delimit (sort seq [order]) delim [last-delim] }}.

[derekperkins]

I like the idea of having a custom sort function that can be chained. That already exists for taxonomies, so I think it could be extracted out and generalized for any of the range queries.

[bep]

Nah, maybe it's just me ... but: I like having automated tests for things; so in this case either:

1. Leave the implementation as is and sort the keys for the test assertions -- and let the end user and the page cache get very confused by their content change in total random order.
2. Sort the keys. Not perfect, but in my eyes MUUUCH better than the above.

Having the user take an extra step to get to what should be default sounds fishy.

[halostatue]

I don’t have a super strong opinion either way. Testability is good, but I think your better argument is related to page caching—it would be bad if, every time your site was regenerated by Hugo, an indeterminate number of pages that used delimit now had different hash values because of this indeterminacy.

I still think that sort would be useful to have on its own.

[derekperkins]

It seems like I'll have to discard the keys and return a slice of the values, otherwise the sort won't be maintained. How does that fit into the context of chained functions?

[bep]

The next function in the chain would have to handle a slice ... but I guess that is what you would want in this case.

Not sure what is correct here, but a sorted slice of values, keys or "key:value" ...

[derekperkins]

I'm thinking that the function signature will be something like:

func Sort(seq interface{}, byField string) (interface{}, error)

A blank field will mean that I sort by the keys, otherwise I'll reflect and sort on whatever field they specify. The return interface{} will be a slice of the values from the original sequence.

[bep]

A little confused by this ... the delimit function returns a string ... so what is the input t Sort? It cannot be the output of Delimit?

[halostatue]

I think that this is actually pretty easy to resolve:

1. If the input sequence is a slice or an Array, delimit should not sort.
2. If the input sequence is a map, delimit should sort the keys and return the values in a stable order based on that sort.

To sort based on the values of the map, we need to implement sort (and possibly a couple of map-specific methods). What I mean is:

{{ delimit (sort (mapkeys .Params.map)) ", " ", and " }}

is the behaviour that I’m suggesting be the default. mapkeys (not yet implemented) extracts the keys from the map and mapvalues (not yet implemented) extracts the values from the map both as arrays. sort sorts a sequence (and, if given a map, acts the same as sort (mapkeys .)), and delimit delimits a sequence (and, if given a map, acts the same as delimit (sort (mapkeys .))).

It may be easier to think of this in pipe format:

{{ . | mapkeys | sort | delimit ", " ", and " }}
{{ . | mapvalues | sort | delimit ", " ", and " }}

[derekperkins]

Ok, I just pushed with a full sort implementation that lets you sort by keys, values or map/struct fields. It has two optional parameters:

1. sortByField
   blank / "" sorts by keys
   "value" sorts by the raw value
   "FieldName" sorts by a map/struct name
2. sortAsc
   "desc" sorts descending and anything else sorts ascending

[derekperkins]

This works great now. The only thing left in my mind as a possible enhancement is to add a third optional parameter that will change sort from returning a slice of the values to returning a slice of the keys. Is that something you'd see yourselves needing?

[bep]

Very hard to read with so many commits. I would recommend you squash them into one and do a push -f.

[derekperkins]

I actually did squash them two commits ago and then didn't push them up correctly, hence the next merge. :) I'm resquashing now.

[derekperkins]

Done :)

[halostatue]

This looks great. I’m putting it into my local version and will be trying it out in places that I’m doing some similar stuff in cabaret.

[derekperkins]

@spf13 Can we get this merged now that it's passing all the tests?

[bep]

Rebase with forced push vs merge, that's the question. I have no idea how the history will look if this was merged into master, but I guess not very pretty. It would be good if you could squash these into one.

[derekperkins]

Ok, I squashed the commits.

[derekperkins]

It seems like it should be pretty clean to merge at this point.

[halostatue]

I have just made changes to cabaret that depend on this and #706, and I’m seeing this work very well in practice (I’m planning on pushing this live on my site soon).

[spf13]

@derekperkins This is a fantastic addition. I'm really happy with the feedback and the way it turned out. Keep the great contributions coming.

Merged as 717f702

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.