Skip to content

Use normal constructor instead of record constructor for i18n messages#290

Merged
jezen merged 4 commits intoyesodweb:masterfrom
L0neGamer:no-record-message
Jul 15, 2025
Merged

Use normal constructor instead of record constructor for i18n messages#290
jezen merged 4 commits intoyesodweb:masterfrom
L0neGamer:no-record-message

Conversation

@L0neGamer
Copy link
Copy Markdown
Contributor

@L0neGamer L0neGamer commented Apr 29, 2025

fixes #289

@L0neGamer L0neGamer marked this pull request as ready for review April 29, 2025 09:05
@parsonsmatt
Copy link
Copy Markdown
Collaborator

This is definitely a breaking change, at least. I'm not sure if this is something we want to do - I don't personally use these and so I don't know what the potential impact will be.

Alternatively, we can introduce another setting that controls if it is record or regular constructors, which is record by default. That way it's not a breaking change, and you get the behavior you want.

@L0neGamer L0neGamer force-pushed the no-record-message branch from a3043c9 to 369b547 Compare May 9, 2025 09:28
@L0neGamer
Copy link
Copy Markdown
Contributor Author

@parsonsmatt I've introduced an options type like I've done previously, exposing setters but not getters nor the internals of the structure so that we can freely manipulate its internals.

I've also extended testing for i18n slightly to test that both cases work.

It also seems there may have been a bug where the constructor prefix wasn't set properly when toCon was used internally. I've fixed this too.

@L0neGamer L0neGamer force-pushed the no-record-message branch from 369b547 to c0a0d67 Compare May 9, 2025 13:16
@L0neGamer L0neGamer force-pushed the no-record-message branch from c0a0d67 to 2f7c31e Compare July 9, 2025 10:46
@L0neGamer L0neGamer force-pushed the no-record-message branch from 2f7c31e to 9cd54e4 Compare July 9, 2025 11:09
@L0neGamer
Copy link
Copy Markdown
Contributor Author

@jezen if you wouldn't mind

@jezen jezen merged commit 7b5dd95 into yesodweb:master Jul 15, 2025
9 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.

Don't use records for Message data structures

3 participants