fix: match date -I error messages with GNU date

[shanmukhateja]

Fixes #4494

[sylvestre]

panic isn't the right approach to manage a user facing error.

With your change:

$ ./target/debug/coreutils date -I@
thread 'main' panicked at 'date:` invalid argument `@` for '--iso-8601'', src/uu/date/src/date.rs:118:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

it should be like GNU:

date: invalid argument '@' for '--iso-8601'
Valid arguments are:
  - 'hours'
  - 'minutes'
  - 'date'
  - 'seconds'
  - 'ns'
Try 'date --help' for more information.

See https:/uutils/coreutils/blob/main/src/uu/date/src/date.rs#L249-L253 as example

Also, it will need a test to make sure we don't regress

[sylvestre]

@shanmukhateja are you still working on it ? :)

[shanmukhateja]

@sylvestre Yes I plan on working on it except I'm not sure how to implement SimpleError struct inside FromStr which returns Self.

In the case of panic, it halts the execution but now when I try using the struct, it complaints of trying to return Result.

Thoughts?

[sylvestre]

You should try a bit more :)
and look how it is done elsewere

[tertsdiepraam]

Have you seen my comment in the issue? You probably don't need to change this function, just make sure that clap gives an error before we even get here.

[shanmukhateja]

@sylvestre @tertsdiepraam I used value_parser to declare valid options. Please let me know if this is correct.

Note: unit test is missing.

Success case:

$ cargo run date -Idate
    Finished dev [unoptimized + debuginfo] target(s) in 0.33s
     Running `target/debug/coreutils date -Idate`
2023-03-15
$ cargo run date -Ihour
    Finished dev [unoptimized + debuginfo] target(s) in 0.31s
     Running `target/debug/coreutils date -Ihour`
2023-03-15T21+05:30

Error case:

$ cargo run date -I@
    Finished dev [unoptimized + debuginfo] target(s) in 0.31s
     Running `target/debug/coreutils date '-I@'`
error: '@' isn't a valid value for '--iso-8601 <FMT>'
  [possible values: date, hour, hours, minute, minutes, ns]

For more information try '--help'

[sylvestre]

look great :)
just a missing test

[sylvestre]

maybe

Suggested change

	_ => panic!("date:` invalid argument `{s}` for '--iso-8601'"),
	_ => unreachable!(),

if we won't go there then

[sylvestre]

ping ?

[shanmukhateja]

Hey, this commit 5e42b58 (#4499) fixes it.

Still missing the unit test. Will ping when I have it working.

[sylvestre]

thanks

[shanmukhateja]

@sylvestre unit tests added.

[sylvestre]

please add the tests here:
https:/uutils/coreutils/blob/main/tests/by-util/test_date.rs
and following their behavior :)

[shanmukhateja]

@sylvestre ..third time's the charm?

[sylvestre]

thanks :)

[shanmukhateja]

Thank you for being patient. @sylvestre

[sylvestre]

oh, don't mention it :)