Open augustoproiete opened 3 years ago
Hi @augustoproiete, as this seems still open, I'd like to give it a go.
@alexandru-valeanu Sure.
Heads-up that we use Spectre.Console for parsing arguments, so there's a little bit of investigation needed to see if the fix can (and should) be done directly in Cake, of if it should be fixed upstream. If you discover that the fix doesn't belong in Cake, you'd need to send a PR to Spectre.Console and the fix in Cake would be just upgrading to a newer version of Spectre.Console.
Thanks for the heads-up.
I'm not sure if this is specified anywhere - but writing constructor like this:
[TaskName("Default")]
public sealed class MyTestTask : FrostingTask<BuildContext>
{
public MyTestTask(Spectre.Console.Cli.IRemainingArguments args)
{
Allows to pick up any additional arguments inside Frosting application.
Wondering if this is documented in Cake or known to users of Cake.
https://github.com/tapika/swupd/blob/develop/cakebuild/CommandLineArgs.cs
I've initially sketched command line support for your own cake frosten build application.
There are several hacks I have applied - hopefully they could be sorted out in both - in Cake and in Spectre.Console.Cli.
Overall problem list:
Cake
: src\Cake.Frosting\Internal\Commands\DefaultCommandSettings.cs
Protected, cannot be used by external application. Maybe should be public instead ?(P.S. '--version' does not work ?)
Spectre.Console.Cli / Cake
: There is a clear need to route some of arguments back to Cake - and need to have a list of cake compatible argument list. I have implemented via reflection (see GetCakeArgs) - but maybe there could be simpler solution ?
Spectre.Console.Cli
: class ParseCommandLineArgs : Command
Spectre.Console.Cli
: See in CommandLineArgumentParser.Parse
Spectre.Console.Cli resides in it's own assembly, Cake.exe in it's own - both don't know anything about user defined cake builder assembly.
And if you want to register your own custom type - that is well protected. I've extracted like this, but maybe these methods should be public instead ?
FieldInfo fi = config.GetType().GetField("_registrar", BindingFlags.NonPublic | BindingFlags.Instance);
var reg = (ITypeRegistrar)fi.GetValue(config);
reg.RegisterLazy(typeof(CommandLineArgs), () => { return newArgs; });
Spectre.Console.Cli
: var helpOpt = new CommandOptionAttribute("-h|--help");
var isMatch = helpOpt.GetType().GetMethod("IsMatch", BindingFlags.NonPublic | BindingFlags.Instance);
IsMatch is not public.
MethodInfo mi = typeof(CommandApp).GetMethod("GetRenderableErrorMessage", BindingFlags.NonPublic | BindingFlags.Static);
Also non-public.
Cake / Spectre.Console.Cli
:: CommandLineArgs args = CommandLineArgumentParser.Parse(_args);
maybe should be provided by Cake as it's own API? But this also maps to what can be done between assemblies.
Hey @tapika are you still interested in taking this one?
Leaving a comment as required @augustoproiete - I'll now wait to see if @tapika responds.
Can / could do it, but as this is kinda hobby, cannot use 24/7 time.
I guess need to integrate changes directly into cake ? Should we aim to improve / patch Spectre.Console.Cli
?
5th item - raised as it's own ticket to Spectre.Console
:
https://github.com/spectreconsole/spectre.console/issues/967
I have opened a PR on spectreconsole which allows command line options to be specified ahead of arguments without incurring the Flags cannot be assigned a value.
error message (see https://github.com/spectreconsole/spectre.console/pull/1029)
This PR, if accepted and merged, would make the following command legal syntax: dotnet cake --tree script.cake
The Flags cannot be assigned a value.
would still be seen for the following command, however dotnet cake script.cake --version=1.2.3
A consequence of the PR being accepted and merged is that fully implementing this cake issue can then be easily done by a further, small code change to spectreconsole. This would introduce a setting (named something like AddFlagsToRemainingArgumentsWhenCannotBeAssigned
) to catch Flags cannot be assigned a value.
errors that would have been thrown for any of the in-built cake options, instead adding the flag and value to the remaining arguments (and avoiding the explicit need for the --
separator).
We'd set AddFlagsToRemainingArgumentsWhenCannotBeAssigned
to true and then the following command would become legal syntax: dotnet cake script.cake --version=1.2.3
(nb. the following would still run dotnet cake script.cake --version
, falling back to existing behaviour and showing the cake version)
I'm going to now prepare this follow-on change to spectreconsole.
Good progress. I do not have so much time to focus on this - working primarily on my own choco fork. Command line argument support is needed only for builder of choco, so it's kinda nice-to-have command line API addition - not primary focus for me.
What I have checked by myself - I have misused Spectre console - there was simpler interface for declaring command line arguments. In choco there is used slightly different API would be nice to harmonize them at some point of time to be the same.
I have raised the following issue on the spectre.console project, which (if done), would fully close off this issue: https://github.com/spectreconsole/spectre.console/issues/1059
I have implemented the change to allow flags to be converted to remaining arguments if they cannot be assigned, and submitted the PR for this: https://github.com/spectreconsole/spectre.console/pull/1102
Once merged and a new version of spectre.console is available, turning this behaviour on should be a 1 line code change in the cake codebase 🙂
FYI: Potentially unrelated, but might be related in future:
Update @augustoproiete - I have asked the maintainers of spectre to prioritise the above PR, so once merged, hopefully I can address this cake issue.
Thanks for chasing @FrankRay78 !
PR https://github.com/cake-build/cake/pull/4142, which fully addresses this issue, is now green and ready for review/merging.
Background
The Cake runner supports a number of arguments, such
--version
to display the version of the runner, or--debug
to launch the script in debug mode.However, this make it impossible for these names to be used as arguments in Cake build scripts. For example, if one wanted to use the argument
version
as an input to the build script:And then run Cake:
They would receive an error:
Flags cannot be assigned a value
Workaround
The current workaround is to use the "remaining arguments" feature when using one of these "reserved" argument names:
Proposal
This issue intends to make the necessary updates to allow existing "flag" arguments to be used in build scripts, so that if the user runs cake with
--version=1.2.3
do not assume the flag--version
and instead execute the Cake script with the argument["version"] = "1.2.3"
.dotnet cake --version
dotnet cake --version=1.2.3
version
argument to1.2.3
This should work for all Cake runner arguments, except for
Verbosity
which is not a "flag" argument.Related: