I was going to revert this today too in order to play with it and test some things out.

This is what I'm not getting. --format is the input format so this should work,right?

'20220604' | into datetime -f '%Y%m%d'

See other comments in #10017

This is what I'm not getting. --format is the input format so this should work,right?

'20220604' | into datetime -f '%Y%m%d'

mm this command gives me

Thu Jan  1 00:00:00 1970

i'm confused now 😆

I get this, which is also wrong.

❯ '20220604' | into datetime -f '%Y%m%d'
Thu, 01 Jan 1970 00:00:00 +0000 (53 years ago)

Oh, I just noticed that it's the same result we're getting. Just in a different format.

mm 🤔
this is annoying...

i meant, it does not have anything to do with this PR, but it means that into datetime or maybe even datetimes in general, are broken 😭

i'm trying to debug now inside of 10055. i'm not sure what's going one yet.

The problem is that it's never getting to this code. The logic is wrong,

    // If input is not a timestamp, try parsing it as a string
    match input {
        Value::String { val, span } => {
            match dateformat {
                Some(dt) => match DateTime::parse_from_str(val, &dt.0) {
                    Ok(d) => {
                        eprintln!("on ok arm");
                        Value::Date { val: d, span: head }
                    }

I think we should land this revert and fix the problem. What's going on is that if you do something like '20220604' | into datetime -f '%Y%m%d', it thinks the 20220604 part is a timestamp and tries to parse it as such. That is the logic problem. Since it thinks it's a time stamp it never even tries to run the --format code. I found the PR and I'm hoping to get @bobhy to take a look.

I think PR #8337 broke it.

I think this fix could be as simple as

1. before this

        if let Ok(ts) = timestamp {

put this

    if dateformat.is_none() {
        if let Ok(ts) = timestamp {

2. then in the actual --format parsing part, do something like this. So, instead of returning the parse_from_str() error, try and use dtparse to parse the string (ignoring the format). Not sure that's the right thing to do.

    match input {
        Value::String { val, span } => {
            match dateformat {
                Some(dt) => match DateTime::parse_from_str(val, &dt.0) {
                    Ok(d) => {
                        eprintln!("on ok arm");
                        Value::Date { val: d, span: head }
                    }
                    // Err(reason) => {
                    //     Value::Error {
                    //         error: Box::new(ShellError::CantConvert { to_type: format!("could not parse as datetime using format '{}'", dt.0), from_type: reason.to_string(), span: head, help: Some("you can use `into datetime` without a format string to enable flexible parsing".to_string()) }),
                    //     }
                    // }
                    Err(_) => {
                        eprintln!("on err arm");
                        match parse_date_from_string(val, *span) {
                            Ok(date) => Value::Date {
                                val: date,
                                span: *span,
                            },
                            Err(err) => err,
                        }
                    }
                },

reverting this because it would still be nice to have this functionality, we just need to fix it.

@fncred now that this fix has been applied, is there any remaining issue? I think all will be clear and straightforward once 0.85 is released and the deprecated --format and --list options are removed from into datetime.

into datetime always produces a date value, never a string. The residual --format that implied output string will be gone.
format date produces a (date format) string. If input is a string, it must be one that dtparse crate recognizes.
There is no switch to let user specify format of input string; if s/he has some very unusual input format, s/he would do string hacking by hand.

@fncred now that this fix has been applied, is there any remaining issue?

@bobhy I took it upon myself to try and fix the problem, but it would be nice if you gave it a once over.

I think all will be clear and straightforward once 0.85 is released and the deprecated --format and --list options are removed from into datetime.

I don't think --format or --list should be deprecated. That PR was reverted. The --format for into datetime is about the INPUT format to parse, not the output format of the datetime. format datetime is what does the output formatting.

I disagree with your second paragraph. There is a switch that let's users specify the format of the input string. That's what https://docs.rs/chrono/0.4.26/chrono/struct.DateTime.html#method.parse_from_str is all about. However, it requires a timezone so we should document that in the parameter help text I think.

You're right, --format was never about output, it was always about format of input (I was thinking of the dear, departed --convert...). And I'm now running an up-to-date build, so I see the latest changes.

So I see the problem mentioned above is still active:

'20220604' | into datetime -f '%Y%m%d'

doesn't work, but

'20210227_135540+0000' | into datetime -f '%Y%m%d_%H%M%S%z'

(from the example) does.

And I think the reason is that you have to use the NaiveDateTime flavor of parse_date_from_string()
to parse an input that does not have a time zone. At least, chrono::DateTime<FixedOffset>::parse_date_from_string() has a disclaimer to that effect.

So, for nu to handle both kinds of string input with --format, it sounds like it would have to check whether the format string contains any of the timezone tokens, %z, and friends, then invoke the correct flavor of parse_date_from_string.

Note that dtparse() already handles strings with and without time zone for us (for the formats it handles, which is many). And you can do string hacking to convert any non-standard format into something dtparse() can handle (caveat: so long as it's fixed format).

So I vote we not make into datetime any more complicated. That's why I suggested to remove --format (and therefore --list). If someone thinks there's real user value in having a builtin command that handles arbitrary date/time formats beyond dtparse, let's refactor it into a date parse command.

And I think the reason is that you have to use the NaiveDateTime flavor of parse_date_from_string() to parse an input that does not have a time zone.

That's what I meant above, when I said, "However, it requires a timezone so we should document that in the parameter help text, I think."

I think we just need to say something like, "try adding a timezone in order to parse", or something like that. Or show people how to tack on _010203+0000 and _%H%M%S%z. I wonder if that's too much. I'd also feel better if we just had a crate that worked better and parsed per provided format.

What's the use case (that dtparse can't handle)?

Having a custom format that you want to change into a datetime. Seems like that's the intention behind ---format anyway.