Skip to content

Make the data conform to RandomAccessCollection #12

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

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

MojtabaHs
Copy link
Contributor

It now supports the entire range of hashable collections, including arrays, ranges, sets, and more

@gordan-glavas-codecons
Copy link
Collaborator

Thanks for contributing!

@gordan-glavas-codecons gordan-glavas-codecons merged commit e2692f0 into globulus:main Sep 5, 2024
@gordan-glavas-codecons
Copy link
Collaborator

@MojtabaHs Is there a reason why you set the Data.Element to conform to Hashable? It's not necessary for the library to work, and historically people have complained about this constraint.

@MojtabaHs
Copy link
Contributor Author

MojtabaHs commented Sep 5, 2024

@MojtabaHs Is there a reason why you set the Data.Element to conform to Hashable? It's not necessary for the library to work, and historically people have complained about this constraint.

I think it is a leftover of my experiments with the enhancements I was going to apply to the library, like applying the pixel-perfect spacing instead of the current ambiguous padding which applies 2 spacings in between and on unintended at the sides of the layout.

I will double-check and take care of that in the next few PRs where I cleaned up and documented the code

@gordan-glavas-codecons
Copy link
Collaborator

@MojtabaHs Is there a reason why you set the Data.Element to conform to Hashable? It's not necessary for the library to work, and historically people have complained about this constraint.

I think it is a leftover of my experiments with the enhancements I was going to apply to the library, like applying the pixel-perfect spacing instead of the current ambiguous padding which applies 2 spacings in between and on unintended at the sides of the layout.

I will double-check and take care of that in the next few PRs where I cleaned up and documented the code

Thanks, I'll hold off on releasing a new version until that is sorted out.

@MojtabaHs
Copy link
Contributor Author

MojtabaHs commented Sep 22, 2024

Thanks, I'll hold off on releasing a new version until that is sorted out.

I have done it in the PR #15 . Also rebased the other PR based on this change.

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