-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Scaffolding: Enable text templates #27565
Conversation
@bricelam Fantastic! I plan to add this to Power Tools, so it can drop the files. A user request I have had in connection with the handlebars templates is the ability supply your own set of "templates", I have enabled this by looking for a .zip file with templates in the project root. Maybe the to level folder could be called "CodeTemplates" iso "Templates" ? |
} | ||
|
||
public override bool HasTemplates(string projectDir) | ||
=> File.Exists(Path.Combine(projectDir, "Templates", "EFCore", "DbContext.t4")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use "CodeTemplates" as base folder name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. That’s what it used to be in EF6. I want to run this design by the MVC scaffolding team too and see if they have any future plans. I hinted at this on dotnet/Scaffolding#606 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ErikEJ Do you have any additional reasons for preferring CodeTemplates? (Other than that's what EF6 used.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is used by the Handlebars templates. But that is most likely also an EF6 carry over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most likely not important. Assume you would never use both and Handlebars could do a breaking change.
src/EFCore.Design/Scaffolding/Internal/TextTemplatingModelGenerator.cs
Outdated
Show resolved
Hide resolved
So, I had a crazy idea to generate an ER diagram using mermaid. Here's my template:
And here's what it generates when I point it at Northwind: NorthwinderDiagram
CATEGORY {
int CategoryId
nvarchar CategoryName
ntext Description
image Picture
}
CUSTOMER {
nchar CustomerId
nvarchar Address
nvarchar City
nvarchar CompanyName
nvarchar ContactName
nvarchar ContactTitle
nvarchar Country
nvarchar Fax
nvarchar Phone
nvarchar PostalCode
nvarchar Region
}
CUSTOMER }o--o{ CUSTOMERDEMOGRAPHIC : CustomerTypes
CUSTOMERDEMOGRAPHIC {
nchar CustomerTypeId
ntext CustomerDesc
}
EMPLOYEE {
int EmployeeId
nvarchar Address
datetime BirthDate
nvarchar City
nvarchar Country
nvarchar Extension
nvarchar FirstName
datetime HireDate
nvarchar HomePhone
nvarchar LastName
ntext Notes
image Photo
nvarchar PhotoPath
nvarchar PostalCode
nvarchar Region
nvarchar Title
nvarchar TitleOfCourtesy
}
EMPLOYEE }o--o| EMPLOYEE : ReportsToNavigation
EMPLOYEE }o--o{ TERRITORY : Territories
ORDER {
int OrderId
money Freight
datetime OrderDate
datetime RequiredDate
nvarchar ShipAddress
nvarchar ShipCity
nvarchar ShipCountry
nvarchar ShipName
nvarchar ShipPostalCode
nvarchar ShipRegion
datetime ShippedDate
}
ORDER }o--o| CUSTOMER : Customer
ORDER }o--o| EMPLOYEE : Employee
ORDER }o--o| SHIPPER : ShipViaNavigation
ORDERDETAIL {
real Discount
smallint Quantity
money UnitPrice
}
ORDERDETAIL }o--|| ORDER : Order
ORDERDETAIL }o--|| PRODUCT : Product
PRODUCT {
int ProductId
bit Discontinued
nvarchar ProductName
nvarchar QuantityPerUnit
smallint ReorderLevel
money UnitPrice
smallint UnitsInStock
smallint UnitsOnOrder
}
PRODUCT }o--o| CATEGORY : Category
PRODUCT }o--o| SUPPLIER : Supplier
REGION {
int RegionId
nchar RegionDescription
}
SHIPPER {
int ShipperId
nvarchar CompanyName
nvarchar Phone
}
SUPPLIER {
int SupplierId
nvarchar Address
nvarchar City
nvarchar CompanyName
nvarchar ContactName
nvarchar ContactTitle
nvarchar Country
nvarchar Fax
ntext HomePage
nvarchar Phone
nvarchar PostalCode
nvarchar Region
}
TERRITORY {
nvarchar TerritoryId
nchar TerritoryDescription
}
TERRITORY }o--|| REGION : Region
|
test/EFCore.Design.Tests/Scaffolding/Internal/ModelCodeGeneratorTestBase.cs
Show resolved
Hide resolved
src/EFCore.Design/Scaffolding/Internal/TextTemplatingModelGenerator.cs
Outdated
Show resolved
Hide resolved
Tests pass locally on Linux. Not sure why they're timing out on CI... |
81009ae
to
ba85b6d
Compare
test/EFCore.Relational.Specification.Tests/TestUtilities/BuildReference.cs
Show resolved
Hide resolved
{ | ||
foreach (var entityType in model.GetEntityTypes()) | ||
{ | ||
// TODO: Should this be handled inside the template? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Hello @bricelam! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Apologies, while this PR appears ready to be merged, I've been configured to only merge when all checks have explicitly passed. The following integrations have not reported any progress on their checks and are blocking auto-merge:
These integrations are possibly never going to report a check, and unblocking auto-merge likely requires a human being to update my configuration to exempt these integrations from requiring a passing check. Give feedback on thisFrom the bot dev teamWe've tried to tune the bot such that it posts a comment like this only when auto-merge is blocked for exceptional, non-intuitive reasons. When the bot's auto-merge capability is properly configured, auto-merge should operate as you would intuitively expect and you should not see any spurious comments. Please reach out to us at fabricbotservices@microsoft.com to provide feedback if you believe you're seeing this comment appear spuriously. Please note that we usually are unable to update your bot configuration on your team's behalf, but we're happy to help you identify your bot admin. |
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public class TextTemplatingService : ITextTemplating, ITextTemplatingEngineHost, IServiceProvider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The concrete class name seems somewhat inconsistent with the interface name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh this was merged but I do agree, some refinements are needed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just because something has been merged doesn't mean it can't be changed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We sure we want to spend our API naming vouchers on internal code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as ITextTemplating
interface which is public has right name, I am fine whatever the internal concrete type ends up being.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that something is Internal doesn't mean it should be of lower quality. We don't review these names because we can change them at any point, not because we don't care
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take great care in my craft--that's why I chose the names I did. But I'm always happy to hear higher-quality name suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The concrete class name looks good to me, we'll discuss the interface names in API review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I’ve been thinking of ways we can improve improve things, and I think splitting the class may help. I’ll take a note to discuss it in API review.
P.S. I apologize for my previous comments. They seem out of character and a bit guarded or defensive to me today. I’ll strive for more openness in the future.
_host.Session = _host.CreateSession(); | ||
try | ||
{ | ||
_host.Session.Add("EntityType", entityType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add option to inject Code
into EntityType templates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the new, more extensible pattern:
<#@ template hostSpecific="true" #>
<#
var serviceProvider = (IServiceProvider)Host;
var code = serviceProvider.GetService<ICSharpHelper>();
This enables users to add T4 templates to their project which will be picked up by
dotnet ef dbcontext scaffold
to generate the DbContext and entity type classes.It will look for templates in the following location. Note that the extension
.t4
is used instead of the usual.tt
so that Visual Studio doesn't try to transform them.Note, I haven't actually written any templates yet--that'll be my next PR. If anyone's eager to start playing with this, the templates would look something like the this: (you'll definitely need to add a few import directives)
Part of #4038
To Do
dotnet new
or using a new command indotnet ef