search

T4MVC / R4MVC

R4MVC is a Roslyn code generator for ASP.NET Core MVC apps that creates strongly typed helpers that eliminate the use of literal strings in many places
Apache License 2.0
157 stars 48 forks source link

Fix #78 : Partial views in sub-directories are not available in Views class #80

Closed Guymestef closed 6 years ago

Guymestef commented 6 years ago

I looked at T4MVC generated code to generate the sub-views classes as T4MVC...

kevinkuszyk commented 6 years ago

@Guymestef thanks for the contribution. Apologies for keeping you waiting a while.

@artiomchi does this look ok to you?

artiomchi commented 6 years ago

Hey @Guymestef, thanks a lot for your contribution!

Sorry for not looking at this sooner. I got sidetracked, and didn't have time to work on the project, but I trying to get back to it.

I've just pulled the code and ran the generator against your branch, and there's one issue with it. The DisplayTemplates and EditorTemplates folders have to be handled differently, since the fields in them have to contain just the names of the views, not the full paths.

If you could update the PR to handle that, we could most definitely bring this in!

Guymestef commented 6 years ago

Hi @artiomchi, I merged develop into my fork and updated the PR

artiomchi commented 6 years ago

Hey @Guymestef

Sorry it's taking so much time for me to to reply. I promise to become better :)

A couple things that I'd suggest: Try run the generator against the host project (set the Tools project as the startup project and run it, it's already configured to do it's thing). Then commit the changes to the host app. This way the PR will also contain the changes that show how your code will affect end user's projects.

In the current case, your PR still changes the Display and Editor templates by wrapping the templates in the ViewsNames class.

I think your WithSubViewsClass method should only be used when IsAspNetTemplateDirectory is false. Otherwise you should run the old code in the source branch.

Also, quite a big chunk of the WithSubViewsClass method seems to be a slightly altered version of the WithViewsClass method (with some lines shuffled around and variables renamed), which brings a bunch of duplicated code. I can either extract the duplicated code in a separate method, or leave that to you :)

Guymestef commented 6 years ago

Hi @artiomchi , I updated the code to keep the Display and Editor templates as before the PR. There are diff on this part in the PR, but it's just a code reorder in the generator that reordered some line in the generated code.

I started to extract some duplicated code. If you want to to do more about this part, feel free to do it ;)

artiomchi commented 6 years ago

Hey @Guymestef. I know I said I'll try handle the PR faster, but got sidetracked, sorry

I made some minor changes to the code, but the basic approach is the same as in the original PR. The generated output is unchanged.

Thanks for your last commit that pushed the host project updates - it makes so much easier to see how the PR will affect the output. I'll try add some contributor guidelines regarding this.

Great job, and massive thanks for your contribution!