Skip to content

Support default offset with dateformat option#13289

Merged
fdncred merged 4 commits intonushell:mainfrom
hqsz:datetime-default-offset-without-timezone
Jul 4, 2024
Merged

Support default offset with dateformat option#13289
fdncred merged 4 commits intonushell:mainfrom
hqsz:datetime-default-offset-without-timezone

Conversation

@hqsz
Copy link
Copy Markdown
Contributor

@hqsz hqsz commented Jul 3, 2024 •
edited
Loading

Description

Fixes #13280. After apply this patch, we can use non-timezone string + format option into datetime cmd

User-Facing Changes

AS-IS (before fixing)

$ "09.02.2024 11:06:11" | into datetime --format '%m.%d.%Y %T'
Error: nu::shell::cant_convert

  × Can't convert to could not parse as datetime using format '%m.%d.%Y %T'.
   ╭─[entry #1:1:25]
 1 │ "09.02.2024 11:06:11" | into datetime --format '%m.%d.%Y %T'
   ·                         ──────┬──────
   ·                               ╰── can't convert input is not enough for unique date and time to could not parse as datetime using format '%m.%d.%Y %T'
   ╰────
  help: you can use `into datetime` without a format string to enable flexible parsing

$ "09.02.2024 11:06:11" | into datetime
Mon, 2 Sep 2024 11:06:11 +0900 (in 2 months)

TO-BE(After fixing)

$ "09.02.2024 11:06:11" | into datetime --format '%m.%d.%Y %T'
Mon, 2 Sep 2024 20:06:11 +0900 (in 2 months)

$ "09.02.2024 11:06:11" | into datetime 
Mon, 2 Sep 2024 11:06:11 +0900 (in 2 months)

Tests + Formatting

If there is agreement on the direction, I will add a test.

After Submitting

@hqsz hqsz mentioned this pull request Jul 3, 2024
Closed
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Jul 3, 2024 •
edited
Loading

What happens if you use a timezone offset? Does it still work?

@hqsz
Copy link
Copy Markdown
Contributor Author

hqsz commented Jul 3, 2024

@fdncred It also works 😄 belows are example

$ "09.02.2024 11:06:11+0000" | into datetime --format '%d.%m.%Y %H:%M:%S%z'
Fri, 9 Feb 2024 11:06:11 +0000 (4 months ago)

@fdncred fdncred added the deprecated:pr-commands (deprecated: too vague) This PR changes our commands in some way label Jul 3, 2024
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Jul 3, 2024

I'm fine with this. A test would be great.

@hqsz
Copy link
Copy Markdown
Contributor Author

hqsz commented Jul 3, 2024

Thanks for your comments! I will add test suites after finishing personal tasks.

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Jul 4, 2024

Tests look good. I added another example and changed the wording to be more specific. Thanks for your help!

@fdncred fdncred merged commit d5e00c0 into nushell:main Jul 4, 2024
@hustcer hustcer added this to the v0.96.0 milestone Jul 5, 2024
@hqsz hqsz deleted the datetime-default-offset-without-timezone branch July 6, 2024 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

deprecated:pr-commands (deprecated: too vague) This PR changes our commands in some way

Projects

None yet

Milestone

v0.96.0

Development

Successfully merging this pull request may close these issues.

Weird error when using into datetime

3 participants

@hqsz @fdncred @hustcer