Skip to content

Conversation

@Finistere
Copy link
Contributor

Hi! Currently, implementing render_section with indexes feature would require writing an equivalent to IndexedBasedReader.
As the logic is likely to be the same all the time, I have added a render_indexed_content_section to handle this.

I'm not really sure if that's the best approach though.

Furthermore there was an inconsistency, except Vec<T>, all other list types like &[T] would simply ignore the indexes feature.

Currently implementing render_section with indexes feature would require
writing an equivalent to `IndexedBasedReader`. As the logic is likely to
be the same all the time, I've added a `render_indexed_content_section`
function instead, to not expose the type and work on a
`ExactSizeIterator`. I'm not really sure if that's the best approach
though.

Furthermore there was an inconsistency, except `Vec<T>`, all other list
types like `&[T]` would simply ignore the `indexes` feature.
@maciejhirsz
Copy link
Owner

This looks like it should be or remain to be an implementation detail, but I'll need to look back at the code to give a better suggestion.

@Finistere
Copy link
Contributor Author

An alternative could be to not expose this function publicly but keep the fixes on &[T] etc. and I copy the relevant code.

It feels a bit weird that I would need to implement an IndexedBasedReader myself, seems like something that ramhorns should manage somehow. Maybe Content should have something like a render_list_section(...) -> impl ExactSizeIterator instead of render_index_section(). This would keep Indexed, Index and IndexedBasedReader as implementation details.

But I haven't worked enough with ramhorns for now to be sure. 🤷

@Finistere
Copy link
Contributor Author

I'm not sure if the indexing capabilities are that relevant to be honest. I'm mostly interested in {{-last}} to make formatting of JSON lists possible. So could be render_list_section(...) -> impl Iterator if only first & last are kept.

@maciejhirsz
Copy link
Owner

Maybe Content should have something like a render_list_section(...) -> impl ExactSizeIterator instead of render_index_section(). This would keep Indexed, Index and IndexedBasedReader as implementation details.

This seems sensible to me. Give me a day to dig into it first. In the mean time having the helper function but it not being public as a fix for slices is also fine.

@Finistere
Copy link
Contributor Author

Nothing urgent on my side, take your time! I can keep using our fork for a while. I just hope to not keep it forever, but seems like that won't be the case. :)

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.

2 participants