GH1184: Added support for manually including/excluding files for tools #1202
Conversation
patriksvensson
commented
Aug 30, 2016
•
patriksvensson
commented
- Query string should support multiple parameters with same name
- Add include/exclude parameter
- Better error messages
- Tests
| { | ||
| foreach (var include in package.Parameters["include"]) | ||
| { | ||
| collection.Add(_globber.GetFiles(include)); |
There was a problem hiding this comment.
Add path to include pattern.
patriksvensson
changed the title
|
@cake-build/team Ok, I think this is ready for review. |
|
Relates to #1184 |
|
@patriksvensson are you in a position to rebase this? And I can try to take a look tonight? |
|
@gep13 Sure, I will do this during the day. |
|
@gep13 Rebased |
| [Theory] | ||
| [InlineData("nuget:?package=Foo&include=/**/*.XML")] | ||
| [InlineData("nuget:?package=Foo&include=**/*.XML")] | ||
| [InlineData("nuget:?package=Foo&include=./**/*.XML")] |
There was a problem hiding this comment.
@patriksvensson I am assuming that the include/exclude is not case sensitive, right?
There was a problem hiding this comment.
Good question. Will add some tests 😄
There was a problem hiding this comment.
The include/exclude is case sensitive due to the globber. How should we proceed?
There was a problem hiding this comment.
We could change the interface for IGlobber to take a PathComparer and move the existing method to an extension method (that uses the default comparer). A breaking change, but something that would be useful in general. Or we could extend globbing to accept regex expressions, like here: https://en.wikipedia.org/wiki/Glob_(programming)
There was a problem hiding this comment.
@patriksvensson hmm, I think Regex might be over complicating it. I think I would prefer to add a breaking change, where the user can control the outcome, without having to dig into regex. @devlead thoughts?
There was a problem hiding this comment.
Yes, agree. I was hoping you would say that 😆
One thing we can do is to add a GlobSettings as the last parameter and start moving other glob-related things there. That would collect all those things at one place.
There was a problem hiding this comment.
I'll say ship case sensitive. And if necessary break next release.
There was a problem hiding this comment.
@devlead I would be ok with that as well.
|
@gep13 Rebased against upstream/develop. |
