Please add support for DateOnly and TimeOnly types #1732

Activity

1. RobThree commented on May 9, 2022

   

   RobThree

   on May 9, 2022

   ContributorAuthor

   More actions

   Also, counter-intuitively (IMHO), when providing defaults the exact opposite fails (testdate1 and testtime1):

   using System.CommandLine;
   using System.CommandLine.NamingConventionBinder;
   
   internal class Program
   {
       public static async Task Main()
       {
           var root = new Command("Root");
   
           var datecommand1 = new Command("testdate1") { Handler = CommandHandler.Create<DateTime>((date) => Console.WriteLine(date)) };
           datecommand1.AddOption(new Option<DateOnly>(new[] { "--date", "-d" }, () => DateOnly.Parse("2999-12-31"), "Test date") { IsRequired = false });
   
           var datecommand2 = new Command("testdate2") { Handler = CommandHandler.Create<DateOnly>((date) => Console.WriteLine(date)) };
           datecommand2.AddOption(new Option<DateOnly>(new[] { "--date", "-d" }, () => DateOnly.Parse("2999-12-31"), "Test date") { IsRequired = false });
   
           var timecommand1 = new Command("testtime1") { Handler = CommandHandler.Create<DateTime>((time) => Console.WriteLine(time)) };
           timecommand1.AddOption(new Option<TimeOnly>(new[] { "--time", "-t" }, () => TimeOnly.Parse("23:59:59"), "Test time") { IsRequired = false });
   
           var timecommand2 = new Command("testtime2") { Handler = CommandHandler.Create<TimeOnly>((time) => Console.WriteLine(time)) };
           timecommand2.AddOption(new Option<TimeOnly>(new[] { "--time", "-t" }, () => TimeOnly.Parse("23:59:59"), "Test time") { IsRequired = false });
   
           var datetimecommand = new Command("testdatetime") { Handler = CommandHandler.Create<DateTime>((datetime) => Console.WriteLine(datetime)) };
           datetimecommand.AddOption(new Option<DateTime>(new[] { "--datetime", "-dt" }, () => DateTime.Parse("2999-12-31T23:59:59"), "Test time"));
   
           var datetimeoffsetcommand = new Command("testdatetimeoffset") { Handler = CommandHandler.Create<DateTimeOffset>((datetimeoffset) => Console.WriteLine(datetimeoffset)) };
           datetimeoffsetcommand.AddOption(new Option<DateTimeOffset>(new[] { "--datetimeoffset", "-dto" }, () => DateTimeOffset.Parse("2999-12-31T23:59:59Z"), "Test time"));
   
           root.AddCommand(datecommand1);
           root.AddCommand(datecommand2);
           root.AddCommand(timecommand1);
           root.AddCommand(timecommand2);
           root.AddCommand(datetimecommand);
           root.AddCommand(datetimeoffsetcommand);
   
           var tests = new[]
           {
               new[] { "testdate1" },
               new[] { "testdate2" },
               new[] { "testtime1" },
               new[] { "testtime2" },
               new[] { "testdatetime" },
               new[] { "testdatetimeoffset" }
           };
   
   
           foreach (var t in tests)
           {
               Console.WriteLine($"Test: {string.Join(" ", t)}");
               await root.InvokeAsync(t);
           }
       }
   }

   Test: testdate1
   1-1-0001 00:00:00
   Test: testdate2
   31-12-2999
   Test: testtime1
   1-1-0001 00:00:00
   Test: testtime2
   23:59
   Test: testdatetime
   31-12-2999 23:59:59
   Test: testdatetimeoffset
   31-12-2999 23:59:59 +00:00
2. elgonzo commented on May 9, 2022

   

   elgonzo

   on May 9, 2022

   Last edited by elgonzo

   Contributor

   More actions

   Also, counter-intuitively (IMHO), when providing defaults the exact opposite fails (testdate1 and testtime1):

   Not really counter-intuitive in my opnion, considering neither DateOnly, TimeOnly, nor DateTime have conversion operators allowing explicit/implicit conversions between these types (as far as i can tell, according to the documentation).

   The behavior i personally find surprising is presented in your first post. Due to the absence of said conversion operators, i would expect testdate1 and testtime1 to fail and not silently ignore the generic type parameters given for the options (DateOnly/TimeOnly).

   (testdate2 and testtime2 failing is quite likely due to System.CommandLine not having built-in support for DateOnly/TimeOnly, yet...)
3. RobThree commented on May 9, 2022

   

   RobThree

   on May 9, 2022

   ContributorAuthor

   More actions

   Not really counter-intuitive in my opnion

   Counter-intuitive in the sense that the cases that failed first when given explicit values now succeed and vice versa.
4. elgonzo commented on May 9, 2022

   

   elgonzo

   on May 9, 2022

   Contributor

   More actions

   Yeah. In this sense, it's counter-intuitve. I was arguing from the perspective that testdate1 and testtime1 from your first post seemingly working is not being intuitive and expected at all, given the conflict between the options being of DateOnly/TimeOnly type and a handler parameter that is of a type (DateTime) which neither DateOnly nor TimeOnly can be explicitly/implictly converted into.
5. jonsequitur commented on May 10, 2022

   

   jonsequitur

   on May 10, 2022

   Contributor

   More actions

   It's worth pointing out that implicit conversions and support for built-in TypeConverters were removed from the core System.CommandLine. (See #1613 for details). Using CommandHandler.Create (i.e. System.CommandLine.NamingConventionBinder) brings in some additional conversion logic that might confuse discussion of the core parsing behavior, especially given you're specifying a different command handler parameter type than the Option<T> type.

   It would be clearer to base the expected versus actual behavior on the GetValueForOption / GetValueForArgument APIs:

   var dateOnlyOption = new Option<DateOnly>("-d");
   var timeOnlyOption = new Option<TimeOnly>("-t");
   
   var cmd = new RootCommand
   {
       dateOnlyOption,
       timeOnlyOption
   };
   
   cmd.Parse("-d 2022-05-09").GetValueForOption(dateOnlyOption).Should().Be( /* ??? */ );
   cmd.Parse("-t 1:39").GetValueForOption(timeOnlyOption).Should().Be( /* ??? */ );

   (testdate2 and testtime2 failing is quite likely due to System.CommandLine not having built-in support for DateOnly/TimeOnly, yet...)

   This is likely the core of the issue.
6. jonsequitur

   added

   Area-Parser and Binder

   bugSomething isn't workingSomething isn't working

   on May 10, 2022
7. jonsequitur

   added this to the 2.0 GA milestone on May 10, 2022
8. jonsequitur

   added a commit that references this issue on May 10, 2022

   Add support for DateOnly and TimeOnly on .Net 6 (#1732). (#1734)

   4dcee4d
9. jonsequitur

   closed this as completedon May 10, 2022