Inconsistent determination of positional Argument when using context.Arguments versus context.Argument

[EdLichtman]

Prerequisites

• I have written a descriptive issue title
• I have searched issues to ensure it has not already been reported

Cake runner

Cake Frosting

Cake version

3.0.0

Operating system

Windows

Operating system architecture

64-Bit

CI Server

No response

What are you seeing?

When I am running a task, and I have an IFrostingContext, I have 4 options to get an argument:

1. ArgumentAliases.Argument(IFrostingContext, string)

• Requires the argument and displays an error if there is none displayed

2. IFrostingContext.ICakeContext.GetArguments()

• Returns a dictionary of collections for all the arguments, which puts the onus on the user to get the argument

3. IFrostingContext.ICakeContext.GetArguments(string)

• Returns a collection of arguments for the provided string, which once again puts the onus on the user to get the Argument

4. ICakeContextExtensions.GetArgument(ICakeContext, string)

• Returns the "Last" instance of an argument provided to the CLI

Since 2 and 3 require the user to do extra work, we'll disregard those, unless they're helpful or informational. So to compare 1 vs 4:

1. Returns and requires an argument to be present. Returns the FIRST instance of the argument.
2. Returns and does not require an argument to be present. Returns the LAST instance of the argument

We need consistency, because there is an actual use-case for both. Sometimes I want to use ArgumentAliases.Argument so that I don't have to write in my own Error Message, and mess with the AnsiConsole in the same way that that function messes with it.

What is expected?

I would expect that when using:

ArgumentAliases.Argument(IFrostingContext, string)

and

ICakeContextExtensions.GetArgument(ICakeContext, string)

I would get the same result.

Steps to Reproduce

Create a task in which you pass in an argument. Then, try to retrieve the argument using the two methods described:
.\build.ps1 --required_single=0 --required_single=1

var foo = context.Argument("required_single");
var bar = context.Arguments.GetArgument("required_single");

See that foo is 0, and bar is 1.

Output log

No response

[EdLichtman]

BTW, I have a commit across streams, but I'm working on some extra integration tests. I'm having trouble running the specific tests I want to run, and having trouble adding new ones. But if you can approve of this, it's a simple modification from FirstOrDefault to LastOrDefault.

Here's my reasoning why we're going Last instead of First:

if you type in --target=A --target=B, we use B as the target, not A. Therefore, to keep it consistent with other places in the application, we should make it always return the last value, that way the value can likewise be overwritten, by whatever process can overwrite it.

While I say it's a "simple modification", I realize this is not a "trivial change" because it completely alters existing behavior for users of frosting who may be using the ArgumentAliases.Argument() function.

[augustoproiete]

Thanks for reporting this @EdLichtman. Returning the value of the last occurrence of the argument is the way to go.

This will be fixed by #4136

[EdLichtman]

@augustoproiete I also have a changelist that includes additional unit/build tests but couldn't get them running locally yet. Could you possibly vet the code for me and add it to yours before you merge in?

[augustoproiete]

@EdLichtman Sure. Point me to it or post a patch here

[EdLichtman]

#4139

[EdLichtman]

@augustoproiete I really appreciate that, even though you also did some work, you gave me the opportunity to get a contribution. It made me very happy!

[cake-build-bot]

🎉 This issue has been resolved in version v3.1.0 🎉

The release is available on:

• GitHub Release
• .NET Tool

Your GitReleaseManager bot 📦🚀