cake-build / resources

Contains different kind of resources such as bootstrappers and configuration files.
MIT License
54 stars 78 forks source link

Adds additional handling around ScriptArgs to be more flexible #12

Closed phil-scott-78 closed 4 years ago

phil-scott-78 commented 8 years ago

This is more of a request for comments, but I think it is a step in the right direction. Currently passing in parameters to a cake script from the powershell bootstrapper isn't consistent, plus kind of weird especially with handling filenames.

Let's take this "build" script


#addin "Cake.FileHelpers"

var target = Argument("target", "Default");
var file = Argument<string>("file");
var someBool = Argument<bool>("someBool", true);
var someString =Argument<string>("someString", "hello!");

Task("Do-the-do")
    .Does(() =>
{
    Information("Bool value? {0}", someBool.ToString());
    Information("someString? {0}", someString);
    var exists = FileExists(file).ToString();
    Information("File {0} exists? {1}", file, exists);    
});

Task("Default")
    .IsDependentOn("Do-the-do");

RunTarget(target);

With the current implementation to run this script with diagnostic verbosity you must run to get this to work (note no equal after verbosity but required for the others)

.\build.ps1 -verbosity diagnostic -file="""file.txt""" -someBool=true

Additionally, if you make any mistake it'll blindly pass those args in cake giving some less than helpful errors because cake expects things differently than the bootstrapper. Some of these error messages actually point you in the wrong direction (see the verbosity help that is outputted)

 .\build.ps1 -verbosity=diagnostic -file="""file.txt"""

you get this output

Preparing to run build script...
Running build script...
Multiple arguments with the same name (verbosity).
Module directory does not exist.

Usage: Cake.exe [script] [--verbosity=value]
            [--showdescription] [--dryrun] [..]

Example: Cake.exe
Example: Cake.exe build.cake --verbosity=quiet

Or if you don't properly quote the file name

.\build.ps1 -verbosity diagnostic -file=file.txt

you get this output

Preparing to run build script...
Running build script...
More than one build script specified.
Module directory does not exist.

This change allows you to run

.\build.ps1 -verbosity diagnostic -file file.txt -someBool

Looks more like powershell and still sends things into cake.exe as expected. File.txt will be properly quoted and someBool will be sent as someBool=true. Not sure if I've caught all the edge cases here especially around what values need to be quoted, but I wanted to throw this out and see what you all think. I'll be away on holiday until the 1st but hopefully I'll be able to check in and answer any questions or get tweaks implemented if this is something valuable.

patriksvensson commented 8 years ago

Perhaps an ever easier solution would be to remove all the PowerShell arguments to the bootstrapper so there's a 1:1 mapping with Cake.

build.ps1 --verbosity=diagnostic --foo=bar

Not saying that we should do that (need some discussion) but our goal is to make the bootstrappers simpler and this would be a step in that direction.

@cake-build/team Any thoughts about this?

phil-scott-78 commented 8 years ago

@patriksvensson - sorry for the delay getting back. Internet connection was worse than expected while traveling. So I tossed the idea around but honestly I couldn't wrap my head around a way to handle it without breaking backwards compatibility with anyone that might be using the current powershell like syntax. All my attempts ended with a huge if statement mess with a nasty combination of string parsing and examining $MyInvocation

The big hurdle I encountered even when trying to rip out the powershell arguments is PS's insistence that it doesn't want to tell you about the quotes that might have been passed in on the command line. So if, for example, you passed in -file="hello.text" then $args is -file=hello.txt. Without the quotes the cake.exe command line parser chokes on the . $MyInvocation.Line does have the quotes, but as I said then you get into the weird world of parsing that out. Much easier if you break backwards compatibility with the current script's parameter binding, but huge headache otherwise.

pascalberger commented 4 years ago

We did some maintenance work on this repo which included removing of the develop branch which closed this PR.

@phil-scott-78 In the meantime Cake .NET Tool has been made the recommended approach for running Cake scripts. If you feel this is still an issue please start a discussion.