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

PR #15 modified perfectly valid enum values: "/" replaced with "_" #16

Open
MJacred opened this issue Jul 23, 2024 · 4 comments · May be fixed by #17
Open

PR #15 modified perfectly valid enum values: "/" replaced with "_" #16

MJacred opened this issue Jul 23, 2024 · 4 comments · May be fixed by #17

Comments

@MJacred
Copy link
Contributor

MJacred commented Jul 23, 2024

@starsep, @stephenafamo: Replacing a "/" in the constant identifier is fine, but NOT in the actual string value, which is perfectly valid syntax, and necessary for some users.

cause: #15

kind ENUM('USt.', 'VSt.', 'USt./VSt.') NOT NULL,
// at version sqlboiler v4.16.2 (strmangle v0.0.6):
const (
	TaxKindUSTVST TaxKind = "USt./VSt."
)


// now:
const (
	TaxKindUSTVST TaxKind = "USt._VSt."
)
@MJacred MJacred changed the title PR #15 broke enum values: "/" replaced with "_" PR #15 modified perfectly valid enum values: "/" replaced with "_" Jul 23, 2024
@starsep
Copy link
Contributor

starsep commented Jul 23, 2024

You are correct. Indeed slash should be kept in the string value.

I won't be able to fix it anytime soon.
In my usecase it's not an issue, only invalid Golang identifier was.

@MJacred
Copy link
Contributor Author

MJacred commented Jul 23, 2024

@starsep: The "/" in identifiers was already escaped by v0.0.6. Your PR only touched ParseEnumVals, which is counterproductive. So the PR could be reverted, no?

@starsep
Copy link
Contributor

starsep commented Jul 23, 2024

That seems unlikely.
I used sqlboiler (I think) v4.16.2 which would generate invalid Golang identifiers.

It's using strmangle v0.0.6: https://github.com/volatiletech/sqlboiler/blob/v4.16.2/go.mod#L32

If you can test that it works correctly in v0.0.6 then indeed revert sounds good.

@MJacred
Copy link
Contributor Author

MJacred commented Jul 23, 2024

This issue's description shows the actual result of running version v4.16.2 as well as the latest sqlboiler master (which includes your change). And I tested multiple times, using a real scenario. As far as I can tell, your change only affects the enum string value

@MJacred MJacred linked a pull request Jul 23, 2024 that will close this issue
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 a pull request may close this issue.

2 participants