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

Automatically set Link metadata on items outside the project folder #1246

Merged
merged 4 commits into from
May 25, 2017

Conversation

dsplaisted
Copy link
Member

Fixes #1115

  • Automatically add Link metadata for Compile, AdditionalFiles, None, Content, and EmbeddedResource items where:
    • Link metadata is not already set, and
    • The item’s FullPath is outside of the project directory
  • If setting the Link metadata automatically, the value will be set to %(LinkBase)\%(RecursiveDir)%(Filename)%(Extension). If LinkBase or RecursiveDir are not defined, then those pieces will be left out of the value. So <Compile Include="..\Shared\**\*.cs" LinkBase="Shared" /> would show the items under the "Shared" folder in solution explorer, instead of the root, and would preserve any heirarchy under the shared folder.
  • Allow opting out of automatically adding Link metadata by setting the SetLinkMetadataAutomatically property to false

@@ -102,6 +102,53 @@ Copyright (c) .NET Foundation. All rights reserved.
<RuntimeFrameworkVersion Condition="'$(RuntimeFrameworkVersion)' == ''">$(_TargetFrameworkVersionWithoutV)</RuntimeFrameworkVersion>
</PropertyGroup>

<ItemGroup Condition="'$(SetLinkMetadataAutomatically)' != 'false'">
<Compile Update="@(Compile)">
<Link Condition="'%(Link)' == '' And !$([System.IO.Path]::GetFullPath('%(FullPath)').StartsWith('$([System.IO.Path]::GetFullPath('$(MSBuildProjectDirectory)'))\'))">*NeedsLink*</Link>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be separate NeedsLink=true metadata? The sentinel feels hacky.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you checked the perf of calling get full path this way on every item?

<Link Condition="'%(Link)' == '*NeedsLink*' And '%(RecursiveDir)' != '' And '%(LinkBase)' == ''">%(RecursiveDir)%(Filename)%(Extension)</Link>
<Link Condition="'%(Link)' == '*NeedsLink*' And '%(RecursiveDir)' != '' And '%(LinkBase)' != ''">%(LinkBase)\%(RecursiveDir)%(Filename)%(Extension)</Link>
<Link Condition="'%(Link)' == '*NeedsLink*' And '%(RecursiveDir)' == '' And '%(LinkBase)' == ''">%(Filename)%(Extension)</Link>
<Link Condition="'%(Link)' == '*NeedsLink*' And '%(RecursiveDir)' == '' And '%(LinkBase)' != ''">%(LinkBase)\%(Filename)%(Extension)</Link>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just use %(RecursiveDir) without checking if it's empty. Seems it would evaluate to the right thing in both cases.

<Link Condition="'%(Link)' == '*NeedsLink*' And '%(RecursiveDir)' == '' And '%(LinkBase)' != ''">%(LinkBase)\%(Filename)%(Extension)</Link>
</EmbeddedResource >
</ItemGroup>

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't LinkBase work inside the project cone as well? I might want to display/copy in/to a different relative path.

@dsplaisted dsplaisted requested review from livarcocc and davkean May 24, 2017 20:19
@dsplaisted dsplaisted changed the base branch from master to release/2.0.0 May 24, 2017 20:37
@dsplaisted
Copy link
Member Author

@MattGertz for approval

Customer scenario

Including files (for compilation, copying to output, etc) that are outside of a project folder in a project. Without this change, those items will not be shown in Solution Explorer by default.

Bugs this fixes:

#1115

Workarounds, if any

Set Link metadata manually on items (this is very hard to figure out how to do if you're including items via a glob).

Risk

Low. This change will cause some items that were included via NuGet packages to show up in solution explorer where they weren't previously displayed. An analysis of packages on NuGet.org indicated that at most 43 packages would be affected by this.

Performance impact

Low.

Is this a regression from a previous update?

No

Root cause analysis:

n/a - This is more of a feature / improved experience than a bug

How was the bug found?

n/a

<Link Condition="'%(Link)' == '' And !$([MSBuild]::ValueOrDefault('%(FullPath)', '').StartsWith($([MSBuild]::EnsureTrailingSlash($(MSBuildProjectDirectory)))))">%(LinkBase)%(RecursiveDir)%(Filename)%(Extension)</Link>
</Content>

<EmbeddedResource Update="@(EmbeddedResource )">
Copy link
Contributor

@nguerrera nguerrera May 24, 2017

Choose a reason for hiding this comment

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

whitespace: @(EmbeddedResource ) -> @(EmbeddedResource)

<EmbeddedResource Update="@(EmbeddedResource )">
<LinkBase Condition="'%(LinkBase)' != ''">$([MSBuild]::EnsureTrailingSlash(%(LinkBase)))</LinkBase>
<Link Condition="'%(Link)' == '' And !$([MSBuild]::ValueOrDefault('%(FullPath)', '').StartsWith($([MSBuild]::EnsureTrailingSlash($(MSBuildProjectDirectory)))))">%(LinkBase)%(RecursiveDir)%(Filename)%(Extension)</Link>
</EmbeddedResource >
Copy link
Contributor

@nguerrera nguerrera May 24, 2017

Choose a reason for hiding this comment

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

whitespace:

@(EmbeddedResource ) -> @(EmbeddedResource)
</EmbeddedResource /> -> </EmbeddedResource>

@dsplaisted
Copy link
Member Author

@nguerrera @livarcocc Can one of you sign off on this?

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

Successfully merging this pull request may close these issues.

4 participants