spectre.console icon indicating copy to clipboard operation
spectre.console copied to clipboard

Simple switches end up on "remaining" list even if parsed properly

Open MiloszKrajewski opened this issue 3 years ago • 2 comments

Information

  • OS: Windows 10
  • Version: 0.36 + 0.36.1-preview0.9
  • Terminal: Cmder / Powershell

Describe the bug

What I mean by simple switch:

I assume we can pass bool values in two ways:

[CommandOption("-a <BOOL>")]
public bool A { get; set; }

[CommandOption("-b")]
public bool B { get; set; }

and use them as:

app -a true -b

By simple switch I mean -b (but NOT -a <bool>)

So, even if parsed properly simple switches are also reported on context.Remaining.Parsed list

To Reproduce

using System;
using System.Linq;
using Newtonsoft.Json;
using Spectre.Console.Cli;

namespace SpectreBug
{
	class Program
	{
		static void Main(string[] args)
		{
			var testArgs = new[] {
				"-a", "true", 
				"-b"
			};
			new CommandApp<MyCommand>().Run(testArgs);
		}
	}

	public class MyCommand: Command<MyCommand.Settings>
	{
		public class Settings: CommandSettings
		{
			[CommandOption("-a <BOOL>")]
			public bool A { get; set; }
			
			[CommandOption("-b")]
			public bool B { get; set; }
		}

		public override int Execute(CommandContext context, Settings settings)
		{
			Console.WriteLine(JsonConvert.SerializeObject(settings));
			var debug = new {
				Parsed = context.Remaining.Parsed.ToDictionary(g => g.Key, g => g.ToArray()),
				Raw = context.Remaining.Raw.ToArray()
			};
			Console.WriteLine(JsonConvert.SerializeObject(debug));
			return 0;
		}
	}
}

Expected behavior

Both A and B are set to true, and context.Remaining.Parsed is empty.

{"A":true,"B":true}
{"Parsed":{},"Raw":[]}

Actual behaviour

Both A and B are set to true, but context.Remaining.Parsed still contains -b like it was not recognized.

{"A":true,"B":true}
{"Parsed":{"b":[null]},"Raw":[]}

Additional context

...or maybe I don't understand how bool options should be defined?

MiloszKrajewski avatar Jan 03 '21 00:01 MiloszKrajewski

@MiloszKrajewski Another edge case! 😅 I will take a look asap!

patriksvensson avatar Jan 04 '21 11:01 patriksvensson

I can reproduce this issue with a simplified code snippet that only contains one boolean CommandOption, as follows:

using System;
using System.Linq;
using Newtonsoft.Json;
using Spectre.Console.Cli;

namespace SpectreBug
{
    class Program
    {
        static void Main(string[] args)
        {
            var testArgs = new[] {
                "-a"
            };
            new CommandApp<MyCommand>().Run(testArgs);
        }
    }

    public class MyCommand : Command<MyCommand.Settings>
    {
        public class Settings : CommandSettings
        {
            [CommandOption("-a <BOOL>")]
            public bool A { get; set; }
        }

        public override int Execute(CommandContext context, Settings settings)
        {
            Console.WriteLine(JsonConvert.SerializeObject(settings));
            var debug = new
            {
                Parsed = context.Remaining.Parsed.ToDictionary(g => g.Key, g => g.ToArray()),
                Raw = context.Remaining.Raw.ToArray()
            };
            Console.WriteLine(JsonConvert.SerializeObject(debug));
            return 0;
        }
    }
}

Expected behaviour

{"A":true}
{"Parsed":{},"Raw":[]}

Actual behaviour

{"A":true}
{"Parsed":{"a":[null]},"Raw":[]}

Additional context

To be doubly sure the above code was parsing the command arg and not relying on a default value of A, I also decorated it with the following attribute [DefaultValue(false)] and obtained the same actual behaviour above.

FrankRay78 avatar Sep 21 '22 15:09 FrankRay78

One thing I noticed here is that if you enable strict parsing, you get the expected behavior.

var app = new CommandApp<MyCommand>();
app.Configure(i => i.UseStrictParsing());
app.Run(testArgs);

I'm not sure if this makes it a bug or expected behavior. But here's the theoretically guilty line if this isn't what should be happening

phil-scott-78 avatar Sep 26 '22 03:09 phil-scott-78

Hello @patriksvensson ,

Could you please clarify the expected behaviour of the following? (the above conversation summarised to save you the overhead)

Given the option -a <BOOL> has been specified on a CommandSettings class and is present on the command line and successfully parsed, should it be present in the CommandContext.Remaining.Parsed collection, when:

  • Strict parsing is not enabled?
  • Strict parsing is enabled?

Currently, it is in the Parsed collection when strict parsing is not enabled, but not in the Parsed collection when strict parsing has been enabled. Thank you

(PS - my interest in clarifying, and then possibly correcting, this issue are the following two cake issues, https://github.com/cake-build/cake/issues/3279 and https://github.com/cake-build/cake/issues/3280, that I'm working on and are likely dependent on the use of parsed arguments)

FrankRay78 avatar Oct 09 '22 15:10 FrankRay78

FYI. I've raised PR https://github.com/spectreconsole/spectre.console/pull/1070 which fixes this issue.

FrankRay78 avatar Nov 16 '22 10:11 FrankRay78