-
Notifications
You must be signed in to change notification settings - Fork 6
Add a mechanism for renaming anonymous structs #204
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw. it'd make sense in sn-bindgen too, in a user-provided way
given Render.Config( | ||
indents = Indentation(0), | ||
indentSize = IndentationSize(2), | ||
// todo: these are currently in a global namespace, which is kinda bad? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't spend enough time with the codebase to have a clue if a struct's renderer knows its FQN, so I went with this for now. It works by a miraculous stroke (of luck) as ByCells
/ByDocumentAndCells
both have the same structure.
val newTypeName = "S" + inlineAnonymousStructures | ||
val newTypeName = summon[Config].anonNameOverrides | ||
.get(structure.name.value) | ||
.fold("S" + inlineAnonymousStructures)( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically these S
names don't happen anymore, but this is here to account for new types being added in future LSP versions and the maintainer not wanting to give them names immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think it's worth being strict here – better to fix the names once during update, than to expose S*
and then have to break source compat renaming them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not completely unreasonable
Thoughts? The names may not be the best, but we can bikeshed them and they're probably still better than S0/S1.