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

Add support for _Imports.razor #354

Merged
merged 2 commits into from
Mar 22, 2019
Merged

Add support for _Imports.razor #354

merged 2 commits into from
Mar 22, 2019

Conversation

ajaybhargavb
Copy link
Contributor

@ajaybhargavb ajaybhargavb commented Mar 21, 2019

dotnet/aspnetcore#6374

This adds support for it but doesn't do everything mentioned here. I plan to do that in separate PRs.

@@ -99,6 +97,8 @@ public VirtualProjectItem(string content)

public override bool Exists => true;

public override string FileKind => FileKinds.ComponentImport;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not being used yet. I plan to use this to add errors for adding code, markup etc in an import document,

Copy link
Member

Choose a reason for hiding this comment

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

It might make more sense to use a document classifier for this. FileKind is something that has to be represented in MSBuild.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean. But document classification happens after parsing and IR lowering. I was hoping to add those errors at one of those layers. This means I'll have to do this in a separate IR pass.

Copy link
Member

Choose a reason for hiding this comment

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

OK thats fine - add this logic to the SDK as well (with tests). Ultimately we want the project system (and msbuild) to tell us how to interpret the files.

Copy link
Member

Choose a reason for hiding this comment

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

After thinking about this more, I think I agree with you that FileKind is the right choice.

@ajaybhargavb ajaybhargavb requested a review from pranavkm March 21, 2019 23:13
@ajaybhargavb
Copy link
Contributor Author

@pranavkm, I had to rename a ViewImports file for an SDK test. Do you have any suggestions on more SDK tests to cover this or whatever we currently have is enough?

@ajaybhargavb ajaybhargavb force-pushed the ajbaaska/imports-razor branch from e6b919e to b679fe1 Compare March 21, 2019 23:48
@pranavkm
Copy link
Contributor

I had to rename a ViewImports file for an SDK test. Do you have any suggestions on more SDK tests to cover this or whatever we currently have is enough?

Should be enough for now. We just need to make sure that features work E2E, it doesn't have to be exhaustive.

Copy link
Contributor

@NTaylorMullen NTaylorMullen left a comment

Choose a reason for hiding this comment

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

👍

@ajaybhargavb
Copy link
Contributor Author

Merging this now. Making the SDK and other changes in a separate PR.

@ajaybhargavb ajaybhargavb merged commit 1470405 into master Mar 22, 2019
@ajaybhargavb ajaybhargavb deleted the ajbaaska/imports-razor branch March 22, 2019 16:20
JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request May 17, 2020
* Added support _Imports.razor

* Feedback
\n\nCommit migrated from dotnet/razor@1470405
dougbu pushed a commit to dougbu/razor-compiler that referenced this pull request Nov 17, 2021
* Added support _Imports.razor

* Feedback
\n\nCommit migrated from dotnet/razor@1470405

Commit migrated from dotnet/aspnetcore@91a383ad3b96
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.

4 participants