johan-v-r / LibSassBuilder

Sass builder for .NET projects
MIT License
99 stars 14 forks source link

Arg options #9

Closed johan-v-r closed 3 years ago

johan-v-r commented 3 years ago

This is an adaption of @JelleHissink PR to support specifying directory and folder exclusion.

JelleHissink commented 3 years ago

I like the tool, the minimal setup without npm. however the msbuild integration just falls short for me.

I noticed you took a different approach... I don't know the plan you have for my PR now? Reckon you will remove it, however for me I mainly have issues that the project just does not rebuild if I don't edit the project file:

Just my 2 cents, it is your project, you are free to take this route. However looking at this PR in the current state the msbuild integration is taking a big step backwards.

JelleHissink commented 3 years ago

I have been looking at your code some more, I think this is the way to go for the tool. And in the future using the command options we might be able to provide some input for CompilationOptions.

However for the integration in msbuild to me it still seems as the only way to go is to determine the file list there. How could we pass the file list? I only see a few options here:

Is the default positional argument going to cause problems in the long run? E.g. what if a list of files is passed, how is the current directory then not scanned?

johan-v-r commented 3 years ago

Yes that's the idea! I see the tool assisting MSBuild, but not depending on it. So yeah passing in a list of filepaths would work great - as I see you've already done in the other PR.

I think when a list of files are provided with -f or something, that would make sense to override the default behavior. In that case the user is specifying the exact file and not directory, or perhaps it should look for that file in the directory...? That can come with your changes.

The compilation options would fit as part of the MSBuild task which we can include, but we should then also allow users to easily disable it in their projects (not everyone might want that).

I'm merging this first so we can build on this further on yours 👍