Skip to content

Conversation

@konsumlamm
Copy link
Collaborator

Fixes #117.

This is a breaking change, as it changes the behaviour of the Read and Show instances and adds an Ord constraint to the Read instances.

@konsumlamm konsumlamm requested a review from treeowl November 1, 2025 13:34
@konsumlamm
Copy link
Collaborator Author

@treeowl what do you think?

@treeowl
Copy link
Collaborator

treeowl commented Nov 18, 2025

I prefer for Read instances to be permissive. The fromList should be optional.

@konsumlamm
Copy link
Collaborator Author

I prefer for Read instances to be permissive. The fromList should be optional.

You mean it should accept fromAscList/fromDescList or fromList?

@treeowl
Copy link
Collaborator

treeowl commented Nov 18, 2025

What I meant is that it should also accept bare list syntax, a la OverloadedLists.

@konsumlamm
Copy link
Collaborator Author

There are currently no IsList instances, but I'm not opposed to adding them (containers and unordered-containersalso have them). In my opinion, bare list syntax should only be accepted if there are IsList instances.

Are you okay with removing support for fromAscList/fromDescList in Read/Show instances?

@treeowl
Copy link
Collaborator

treeowl commented Dec 3, 2025

fromAscList and fromDescList seem pretty interesting to support properly, but probably not very useful. So yes, get rid of them.

(Proper behavior would be to produce parse failure if they're not actually ascending/descending, but Read doesn't have the sort of error handling mechanisms needed to make that helpful.)

@konsumlamm konsumlamm merged commit 4133c64 into master Dec 4, 2025
26 checks passed
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.

Read instances are somewhat unsafe

3 participants