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

feat(generator): support custom default casing for client properties #877

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

lxxonx
Copy link
Contributor

@lxxonx lxxonx commented Jan 23, 2024

Change Summary

model UserProfile {
  ...
}

this code generate prisma class attributes like prisma.userprofile, which is sometimes very hard to read
i suggest having default attribute name as prisma.user_profile with using jinja2_strcase package.

Checklist

  • Unit tests for the changes exist
  • Tests pass without significant drop in coverage
  • [ ] Documentation reflects changes where applicable
  • Test snapshots have been updated if applicable

Agreement

By submitting this pull request, I confirm that you can use, modify, copy and redistribute this contribution, under the terms of your choice.

@RobertCraigie
Copy link
Owner

RobertCraigie commented Jan 23, 2024

Thanks! Unfortunately, this is a breaking change so I'd like to first introduce this using a config option, e.g.

generator client {
  provider = "prisma-client-py"
  client_casing = "snake_case"
}

I'm also a little hesitant to introduce this new dependency, would it be feasible to do this without that? or at least using a popular package for changing case?

It's also worth noting that longer term, I want to support customising this more freely, e.g. (structure still TBD)

/// @Python(instance_name: 'user_foo')
model UserFoo {
  // ...
}

@RobertCraigie RobertCraigie changed the title support Prisma client class' default attributes as snake case feat(generator): support custom default casing for client properties Feb 5, 2024
Copy link
Owner

@RobertCraigie RobertCraigie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't fully reviewed yet but love the direction!

return match.group(1)[0].upper() + match.group(1)[1:]

input_str = to_camel_case(PASCAL_RE.sub(_replace_fn, input_str))
return input_str[0].upper() + input_str[1:] if len(input_str) != 0 else input_str
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: where did you get these functions from? did you come up with it yourself?

I'm only asking because, if you copied these from somewhere, it would be great to add a link to that place in case these need to be updated in the future.

ACRONYM_RE = re.compile(r'([A-Z\d]+)(?=[A-Z\d]|$)')
PASCAL_RE = re.compile(r'([^\-_]+)')
SPLIT_RE = re.compile(r'([\-_]*[A-Z][^A-Z]*[\-_]*)')
UNDERSCORE_RE = re.compile(r'(?<=[^\-_])[\-_]+[^\-_]')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would be great to add test cases for all these functions!

Comment on lines 762 to 768
if isinstance(config, Config) and config.client_casing == ClientCasing.camel_case:
return to_camel_case(self.name)
elif isinstance(config, Config) and config.client_casing == ClientCasing.pascal_case:
return to_pascal_case(self.name)
elif isinstance(config, Config) and config.client_casing == ClientCasing.snake_case:
return to_snake_case(self.name)
return self.name.lower()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it would be great for this to exhaustively match the enum, you can do this by using our assert_never() helper function. Here's an example from another part of the codebase

        if value == EngineType.binary:
            return value
        elif value == EngineType.dataproxy:  # pragma: no cover
            raise ValueError('Prisma Client Python does not support the Prisma Data Proxy yet.')
        elif value == EngineType.library:  # pragma: no cover
            raise ValueError('Prisma Client Python does not support native engine bindings yet.')
        else:  # pragma: no cover
            assert_never(value)

@RobertCraigie
Copy link
Owner

FYI I cherry picked your change case util functions into a separate PR as I needed them for something else, #918.

I tried to rebase your PR for you to resolve the merge conflicts but it looks like I don't have permission to push to your fork, could you open a separate PR with edit perms and from a different branch? (i.e. a branch that isn't main on your fork)

@lxxonx
Copy link
Contributor Author

lxxonx commented Mar 12, 2024

New PR has opened with edit permission https://github.com/RobertCraigie/prisma-client-py/pull/922
Please check it out and let me know if you need anything for help :)

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