Update default Razor search paths to include ~/[PagesRoot]/Shared #6783
Conversation
| options.ViewLocationExpanders.Add(new PageViewLocationExpander()); | ||
| } | ||
|
|
||
| private static string CombinePath(string path1, string path2) |
There was a problem hiding this comment.
We want to preserve path separators (they are all meant to be forward slashes). Path.Combine canonicalizes paths to use OS dependent path separators.
| } | ||
|
|
||
| var rootDirectory = _pagesOptions.RootDirectory; | ||
| Debug.Assert(!string.IsNullOrEmpty(rootDirectory)); |
There was a problem hiding this comment.
These shouldn't be asserts, they are clearly possible.
There was a problem hiding this comment.
We disallow these in the property setter (https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.RazorPages/RazorPagesOptions.cs#L32), but sure, we can throw for extra goodness.
| options.PageViewLocationFormats.Add("/Views/Shared/{0}" + RazorViewEngine.ViewExtension); | ||
|
|
||
| options.ViewLocationFormats.Add(pagesSharedDirectory); | ||
| options.AreaViewLocationFormats.Add(pagesSharedDirectory); |
There was a problem hiding this comment.
Wait so we're making pages/shared searchable from views too?
There was a problem hiding this comment.
Yeah, that's how @DamianEdwards wants this. The intent is to allow folks to migrate from Views -> Pages while keeping things working.
|
|
||
| namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal | ||
| { | ||
| public class RazorPagesRazorViewEngineOptionsSetupTest |
There was a problem hiding this comment.
Needs two functional tests. (View -> Pages/Shared), (Page -> Pages/Shared)
I don't think there's anywhere else in the system we unit test options setups 😆
| return path1 + path2 + RazorViewEngine.ViewExtension; | ||
| } | ||
|
|
||
| return path1 + "/" + path2 + RazorViewEngine.ViewExtension; |
There was a problem hiding this comment.
since path1 is always _pagesOptions.RootDirectory do we guarantee the absence of a trailing slash? or do we allow that?
There was a problem hiding this comment.
That's what the check in line #47 does, no?
pranavkm
force-pushed
the
prkrishn/6604
branch
from
2b8b243 to
b8f61fd
Compare
|
🆙 📅 |
Fixes #6604