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

Cleanup explicit EmbeddedResource items from System.Windows.Forms.csproj #49

Closed
Shyam-Gupta opened this issue Nov 15, 2018 · 4 comments · Fixed by #8371
Closed

Cleanup explicit EmbeddedResource items from System.Windows.Forms.csproj #49

Shyam-Gupta opened this issue Nov 15, 2018 · 4 comments · Fixed by #8371
Labels
enhancement Product code improvement that does NOT require public API changes/additions help wanted Good issue for external contributors

Comments

@Shyam-Gupta
Copy link
Member

Feedback from EricStj:

Replace all these explicit items with a convention. Seems that you should be able to come up with a globbing convention for these and update the reading code where it doesn't align with the convention (or update items when the convention is insufficient).

@merriemcgaw merriemcgaw added enhancement Product code improvement that does NOT require public API changes/additions help wanted Good issue for external contributors labels Dec 6, 2018
@merriemcgaw merriemcgaw added this to the Future milestone Dec 6, 2018
@elachlan
Copy link
Contributor

elachlan commented Dec 9, 2022

@dreddy-work do the unit/integration tests cover this? I started looking and its relatively simple to change to use globbing.

For System.Windows.Forms.Design, I think this works.

  <!-- Embedded Resources are globbed based on their location. -->
  <ItemGroup>
    <EmbeddedResource Include="Resources\System\ComponentModel\Design\*.ico">
      <Generator>PublicResXFileCodeGenerator</Generator>
      <LogicalName>$System.ComponentModel.Design.([System.String]::Copy('%(FileName)')).ico</LogicalName>
    </EmbeddedResource>
    <EmbeddedResource Include="Resources\System\WinForms\Design\Behavior\*.ico">
      <Generator>PublicResXFileCodeGenerator</Generator>
      <LogicalName>$System.Windows.Forms.Design.Behavior([System.String]::Copy('%(FileName)')).ico</LogicalName>
    </EmbeddedResource>
    <EmbeddedResource Include="Resources\System\WinForms\Design\*.ico">
      <Generator>PublicResXFileCodeGenerator</Generator>
      <LogicalName>$System.ComponentModel.Design([System.String]::Copy('%(FileName)')).ico</LogicalName>
    </EmbeddedResource>
    <EmbeddedResource Include="Resources\System\WinForms\Design\MoverGlyph.bmp">
      <LogicalName>System.Windows.Forms.Design.Behavior.MoverGlyph</LogicalName>
    </EmbeddedResource>
  </ItemGroup>

@dreddy-work
Copy link
Member

@elachlan, i doubt we have full test coverage here. Appreciate adding some unit tests. Soem example usage are in this file : https://github.com/dotnet/winforms/blob/main/src/System.Windows.Forms/src/System/Windows/Forms/DataGridViewHeaderCell.cs

@dreddy-work
Copy link
Member

Unfortunately the sources that access these bitmaps are behind closed source but the way they accessing these resources is similar to what i pasted above.

ghost pushed a commit that referenced this issue Dec 13, 2022
…sign.csproj` (#8358)

For `System.Windows.Forms.Design` this uses globbing to add the Embedded Resources instead of explicit items.

If this goes okay, I will look into the rest of the projects in the solution.

Related: #49

Reference:
https://stackoverflow.com/questions/46584499/how-to-add-a-glob-for-resx-files-for-new-sdk-csproj-file
@elachlan
Copy link
Contributor

@Tanya-Solyanik is the team okay with renaming resource files to allow for easier globbing? Some of the LogicalName values do not match the file names. Mostly the files located in Resources\System\Windows\Forms\ToolStrip\

So Resources\System\Windows\Forms\ToolStrip\imagetoolstripmenuitem.ico => Resources\System\Windows\Forms\ToolStrip\image.ico

@ghost ghost added the 🚧 work in progress Work that is current in progress label Dec 13, 2022
@ghost ghost removed the 🚧 work in progress Work that is current in progress label Dec 15, 2022
@ghost ghost removed this from the Future milestone Dec 15, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Product code improvement that does NOT require public API changes/additions help wanted Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants