Skip to content

Add custom name-rewriter option#189

Open
myieye wants to merge 2 commits into
efcore:mainfrom
myieye:feat/42-custom-convention
Open

Add custom name-rewriter option#189
myieye wants to merge 2 commits into
efcore:mainfrom
myieye:feat/42-custom-convention

Conversation

@myieye

@myieye myieye commented Feb 23, 2023

Copy link
Copy Markdown

fixes #42

Adds a custom-mapping name-rewriter.

@roji roji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, looks good! Just some very minor nits.

Comment thread EFCore.NamingConventions.Test/RewriterTest.cs Outdated
@mphelt

mphelt commented Apr 26, 2023

Copy link
Copy Markdown

Hi, I don't know if I may suggest anything at this point.
Have you considered storing Func<CultureInfo, INameRewriter> _createRewriter instead of Func<string, string>? _customMapper ?

With such field another extension method could be added:
public static DbContextOptionsBuilder<TContext> UseCustomNamingConvention<TContext>(this DbContextOptionsBuilder<TContext> optionsBuilder, Func<CultureInfo, INameRewriter> createRewriter)
that would allow to easily inherit from existing rewriters:
public class RemoveSuffixSnakeCaseNameRewriter : SnakeCaseNameRewriter { ... }.

CustomNameRewriter would then be instantiated in NamingConventionSetPlugin through such factory method and not directly.

@myieye myieye force-pushed the feat/42-custom-convention branch from 53ec0df to d508cf5 Compare May 30, 2023 07:39
@myieye myieye force-pushed the feat/42-custom-convention branch from d508cf5 to 62d44d2 Compare May 30, 2023 07:43
@myieye

myieye commented May 30, 2023

Copy link
Copy Markdown
Author

@roji Finally got around to incorporating your feedback

@mphelt certainly worth considering, but I'm not willing to put any more time into this currently. Sorry.

@myieye

myieye commented Oct 3, 2023

Copy link
Copy Markdown
Author

@roji can this be merged?

@barba3

barba3 commented Jul 4, 2025

Copy link
Copy Markdown

@roji do you have any objection with merging this PR? - The design in my head looks exactly like @myieye's.

Also, @mphelt's suggestion makes sense to me too, but in my humble opinion, it can be in a different PR.

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.

User-defined rewriting conventions

4 participants