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

Allow Cultures Within Names For EmbeddedResources #5824

Merged

Conversation

benvillalobos
Copy link
Member

@benvillalobos benvillalobos commented Oct 21, 2020

Fixes #3064

Allows a workaround for those that want to embed resources that have cultures within their filenames.

The Workaround

Add the WithCulture metadata to your EmbeddedResource and set it to false.

<EmbeddedResource Include="a.cs.b" WithCulture="false"/>
<EmbeddedResource Include="light.sms.text" WithCulture="false" />
<EmbeddedResource Include="Resources.en.xml" WithCulture="false" />
<EmbeddedResource Include="some.cs.template" WithCulture="false" />

@benvillalobos benvillalobos changed the title Allow Cultures Within Names EmbeddedResources Allow Cultures Within Names For EmbeddedResources Oct 22, 2020
@marcpopMSFT marcpopMSFT added this to In progress in 16.9 via automation Oct 22, 2020
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

Tests?

src/Tasks/CreateVisualBasicManifestResourceName.cs Outdated Show resolved Hide resolved
src/Tasks/AssignCulture.cs Outdated Show resolved Hide resolved
src/Tasks/CreateCSharpManifestResourceName.cs Outdated Show resolved Hide resolved
src/Tasks/Culture.cs Outdated Show resolved Hide resolved
@drewnoakes
Copy link
Member

@davkean is this related to dotnet/project-system#1553?

Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

I think the logic works now, thanks!

src/Tasks/Microsoft.Common.CurrentVersion.targets Outdated Show resolved Hide resolved
src/Tasks/Culture.cs Outdated Show resolved Hide resolved
It's surprisingly difficult to find a good name that means "workaround for those that want to name their files with a culture but have those files not be treated as culture-specific files"
16.9 automation moved this from In progress to Reviewer approved Oct 27, 2020
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

Looks good

src/Tasks/AssignCulture.cs Outdated Show resolved Hide resolved
src/Tasks/Culture.cs Outdated Show resolved Hide resolved
@benvillalobos
Copy link
Member Author

benvillalobos commented Nov 6, 2020

NTS: CreateVisualBasicManifestResourceName_Tests.RootnamespaceWithCulture_RetainCultureInFileNamefails only on OSX mono for some reason?

Microsoft.Build.UnitTests.CreateVisualBasicManifestResourceName_Tests.RootnamespaceWithCulture_RetainCultureInFileName
Shouldly.ShouldAssertException : Shouldly uses your source code to generate its great error messages, build your test project with full debug information to get better error messages\nThe provided expression\n    should be\n\"RootNamespace.File.cs.cshtml\"\n    but was\n\"RootNamespace.Subfolder\\File.cs.cshtml\"\n    difference\nDifference     |       |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |        \n               |      \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/       \nIndex          | ...  14   15   16   17   18   19   20   21   22   23   24   25   26   27   28   29   30   31   32   33   34   ...  \nExpected Value | ...  F    i    l    e    .    c    s    .    c    s    h    t    m    l                                       ...  \nActual Value   | ...  S    u    b    f    o    l    d    e    r    \\    F    i    l    e    .    c    s    .    c    s    h    ...  \nExpected Code  | ...  70   105  108  101  46   99   115  46   99   115  104  116  109  108                                     ...  \nActual Code    | ...  83   117  98   102  111  108  100  101  114  92   70   105  108  101  46   99   115  46   99   115  104  ...  \n\nDifference     |       |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |    |   \n               |      \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \\|/  \nIndex          | ...  17   18   19   20   21   22   23   24   25   26   27   28   29   30   31   32   33   34   35   36   37   \nExpected Value | ...  e    .    c    s    .    c    s    h    t    m    l                                                      \nActual Value   | ...  f    o    l    d    e    r    \\    F    i    l    e    .    c    s    .    c    s    h    t    m    l    \nExpected Code  | ...  101  46   99   115  46   99   115  104  116  109  108                                                    \nActual Code    | ...  102  111  108  100  101  114  92   70   105  108  101  46   99   115  46   99   115  104  116  109  108  
  at Shouldly.ShouldBeStringTestExtensions.ExecuteAssertion (Shouldly.Internals.Assertions.IAssertion assertion, System.Func`1[TResult] customMessage) [0x0002b] in <cd631af7e9d7403ea1721c3ce03185bf>:0 
  at Shouldly.ShouldBeStringTestExtensions.ShouldBe (System.String actual, System.String expected, System.Func`1[TResult] customMessage, Shouldly.StringCompareShould options) [0x0000d] in <cd631af7e9d7403ea1721c3ce03185bf>:0 
  at Shouldly.ShouldBeStringTestExtensions.ShouldBe (System.String actual, System.String expected, Shouldly.StringCompareShould options) [0x00000] in <cd631af7e9d7403ea1721c3ce03185bf>:0 
  at Shouldly.ShouldBeStringTestExtensions.ShouldBe (System.String actual, System.String expected) [0x00000] in <cd631af7e9d7403ea1721c3ce03185bf>:0 
  at Microsoft.Build.UnitTests.CreateVisualBasicManifestResourceName_Tests.RootnamespaceWithCulture_RetainCultureInFileName () [0x00018] in <55cde099c8a64ebf88ba154dfeaee7ad>:0 
  at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
  at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0006a] in <ba70b91736bd40cb990a357097dba9c3>:0 

should be\n\"RootNamespace.File.cs.cshtml\"\n but was\n\"RootNamespace.Subfolder\\File.cs.cshtml\"


mono works on linux. install in WSL and try it there.

run tests on master to make sure things are fine.

find something that explicitly on mono and fails on everything else and enable it, just to check

@benvillalobos benvillalobos merged commit 7f18297 into dotnet:master Nov 23, 2020
16.9 automation moved this from Reviewer approved to Done Nov 23, 2020
@Forgind
Copy link
Member

Forgind commented Nov 23, 2020

Are you handling slashes correctly on non-Windows?

@benvillalobos
Copy link
Member Author

benvillalobos commented Nov 23, 2020

@Forgind I believe that's handled before it gets to these functions.

This PR had a failing test because we called the API directly without fixing the file path that included a subfolder. So the options were either to remove the subfolder or fix the file path before the call. There was another function in the visual basic tests that called FixFilePath. so I could have done that. Ultimately, the subfolder wasn't a relevant part of the test so I removed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
16.9
  
Done
Development

Successfully merging this pull request may close these issues.

File.cs.cshtml cannot become EmbeddedResource
4 participants