Msbuild: Allow "Exclude" as an alias for "Remove"

Created on 1 Sep 2016  路  11Comments  路  Source: dotnet/msbuild

Background

MSBuild now allows developers to remove an item statically via the use of "Remove", previously you could only do this within a target.

<ItemGroup>
    <Compile Include="*.cs" Exclude="DoNotCompile.cs" />
    <Compile Remove="AlsoDoNotCompile.cs" />
</ItemGroup>

This is quite powerful and allows you to remove an item statically, without needing to change the original item declaration. For example, a wildcard could be implicitly imported in a props file, and the Remove in a project file.

Problem

The name "Remove" makes sense in the context that there's an existing feature, previously we just allowed it within a target, we now allow it everywhere.

In terms of symmetry of terms however, the name does not make sense. You "add" and "remove" files from a list, or "include" and "exclude" files from a list. You do not "add" and "exclude" files from a list or "include" and "remove" files from a list.

Given existence of "Include" already, naturally developers are going to gravitate towards using "Exclude" by itself (I personally ran into this), only to be told that it's not valid syntax.

Proposal

Based on above, we should allow the use of "Exclude" in the same places that you can "Remove", ie the above example could become:

<ItemGroup>
    <Compile Include="*.cs" Exclude="DoNotCompile.cs" />
    <Compile Exclude="AlsoDoNotCompile.cs" />
</ItemGroup>

Or in a target:

<Target Name="RemoveFiles">
    <Compile Exclude="AlsoDoNotCompile.cs" />
</Target>

This means when Exclude is on the same item as a Include, it excludes items from the Include. When Exclude is by itself, it excludes items from the current list of all items.

Compatibility

Given it has never been valid to use 'Exclude' by itself without an 'Include' - this change should not have an impact on compatibility.

This issue was filed on behalf of feedback by @shanselman.

Language backlog

Most helpful comment

What if we wanted to support removing all items except the ones that match a pattern? Currently we don't support this, but if we did I'd expect it to look like this:

<Compile Remove="obj\**\*.*" Exclude="obj\generated\*.cs" />

That would mean to remove all compile items from the obj directory except .cs files in obj\generated.

But if Exclude can be an alias for Remove, then this could start to get confusing. What do you think?

All 11 comments

tag @Sarabeth-Jaffe-Microsoft @rainersigwald @cdmihai and @AndyGerlicher

What if we wanted to support removing all items except the ones that match a pattern? Currently we don't support this, but if we did I'd expect it to look like this:

<Compile Remove="obj\**\*.*" Exclude="obj\generated\*.cs" />

That would mean to remove all compile items from the obj directory except .cs files in obj\generated.

But if Exclude can be an alias for Remove, then this could start to get confusing. What do you think?

Very good point @dsplaisted. I'd be for having Exclude scoped down to mean filtering the applicability of the current item operation. And we can enable it for all item operations (include, remove, update, keepmetadata, updatemetadata).

If exclude is the only attribute present in the item element, then it would be interpreted as an exclude for a global Update. See #955

I'm not sure what the right thing to do is. I think the request coming from @shanselman is reasonable, and is how people would generally expect it to work. When you think about the details and edge cases though, it starts to get confusing.

If exclude is the only attribute present in the item element, then it would be interpreted as an exclude for a global Update. See #955

This follows naturally from the existing behavior and the changes we are making, but I think it ends up in a place that would be confusing, or at least not the expected behavior for most people.

I'm not sure about backward compatibility but I think it would be clearer if had "Except" instead of "Exclude" to mean filtering the applicability of the current item operation. Then we could use "Exclude" instead of "Remove" or allow both

<ItemGroup>
    <Compile Include="*.cs" Except="DoNotCompile.cs" />
    <Compile Exclude="AlsoDoNotCompile.cs" />
</ItemGroup>
<Compile Exclude="obj\**\*.*" Except="obj\generated\*.cs" />

I like @cannn's suggestion. Except seems like a more suitable term for modifying the main item operation. Exclude could then be aliased to Remove as a main item operation. Back-compat would be fine, since we'd bake in the rule that Exclude can only appear besides Include, in which case it's aliased to Except. @Microsoft/msbuild-maintainers for more opinions.

<Compile Include="*.cs" Except="DoNotCompile.cs" />
<Compile Remove="**/bin/**/*.cs" Except="DoNotRemoveMe.cs" />

<!-- Exclude as an alias to Remove -->
<Compile Exclude="**/bin/**/*.cs" Except="DoNotRemoveMe.cs" />

I agree that Except is a better name for what Exclude does. But I'm not in favor of adding it as an alias now for language-complexity reasons. Under what circumstances should the API choose one over the other when manipulating a project? How would a user familiar with the current rules interpret Except? And what does the error look like if you have

<Compile Remove="**/obj/**" Exclude="**/bin/**/*.cs" Except="DoNotRemoveMe.cs" />
  • API favor: We'd champion Except as the new Exclude, so we'd change the APIs to use Except instead of today's Exclude.
  • What if both Exclude and Except appear for an Include operation. We'd issue a warning saying the preferred operation modifier is Except, but we accept both so we merge the two. Or just error.

What if Add=".." could be used instead of Include? Then the meaning of other existing attributes wouldn't need to change and the verbs would be Add, Update and Remove with Include staying around for compatibility.

At this point, I believe folks are used to the way things work and changing this would be a breaking change. Closing.

It wouldn't need to be a breaking change if Exclude and Remove were both supported, right?

Was this page helpful?
0 / 5 - 0 ratings