Skip to content
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

Allow to name the constants of a string union by configuration #23

Closed

Conversation

Vampire
Copy link
Contributor

@Vampire Vampire commented Sep 17, 2024

This PR is a follow-up to #22, only the top-most commit is to be reviewed.

With this PR you can manually map string union values to identifier names
using the Karakum configuration file.

@sgrishchenko
Copy link
Contributor

I like the idea, but I am afraid of cases when you have two enums with the same name but in different packages.
I would propose another strategy. Let’s introduce a new extension point with signature: (node: TNode, context: ConverterContext) => string | null (like NameResolver). With this approach you will have full control over AST, and you can select different strategies for renaming even if you have multiple enums with the same name.

@Vampire Vampire force-pushed the configurable-string-union-names branch from 7a937ac to 32ed6f8 Compare September 22, 2024 22:51
@Vampire
Copy link
Contributor Author

Vampire commented Sep 22, 2024

Probably better, yeah, the other just seemed easier. :-D
Well, in the end it was not really.
See updated code :-)

@Vampire Vampire force-pushed the configurable-string-union-names branch from 32ed6f8 to 971acca Compare September 23, 2024 19:20
@Vampire Vampire force-pushed the configurable-string-union-names branch from 971acca to 166e764 Compare September 23, 2024 20:02
@Vampire
Copy link
Contributor Author

Vampire commented Jan 14, 2025

Superseded by bbab142

@Vampire Vampire closed this Jan 14, 2025
@Vampire Vampire deleted the configurable-string-union-names branch January 16, 2025 10:53
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