search

volatiletech / strmangle

Support library for sqlboiler
BSD 3-Clause "New" or "Revised" License
8 stars 13 forks source link

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

Open MJacred opened 1 month ago

MJacred commented 1 month ago

@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."
)
starsep commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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