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

Fix bug when field gets converted to blacklisted word #36

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

a1d4r
Copy link
Contributor

@a1d4r a1d4r commented Jun 7, 2021

I found out that if a field name contains upper-case letters, then the underscore won't be appended to the resulting field name if it is reserved (blacklisted, in terms of the project). For example, one might expect ID to be converted to id_ but it appears as id in the model.

The solution is to change the order of string operations:

  1. Convert unicode
  2. Get rid of non-word characters
  3. Make sure the field name starts with a letter
  4. Convert to camel case
  5. Check if the field name is blacklisted

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 98.375% when pulling 2b7d331 on a1d4r:fix_blacklist into 62d77e6 on bogdandm:master.

@bogdandm
Copy link
Owner

bogdandm commented Jun 8, 2021

Thank you, this is pretty good fix. I think there is one more minor case when after all conversions class name could shadow other global definitions in camel case (like True/False/Exception). But it could be fixed sometime in the future, it's quite rare to see field named liked this. I will look closely to this later.

@bogdandm bogdandm merged commit a528def into bogdandm:master Jun 8, 2021
@bogdandm
Copy link
Owner

bogdandm commented Jun 8, 2021

Currently I could not publish new minor version because of broken/outdated Travic-CI pipeline 😞
https://travis-ci.org/github/bogdandm/json2python-models/jobs/773948039

I will migrate this project to https://www.travis-ci.com/ (or maybe github-actions) in few weeks and than I will make a new release.

@a1d4r a1d4r deleted the fix_blacklist branch June 8, 2021 18:13
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.

3 participants