Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom datetime format through strftime strings #9500

Merged
merged 18 commits into from Jun 23, 2023

Conversation

fnordpig
Copy link
Contributor

@fnordpig fnordpig commented Jun 21, 2023

  • improves usability of datetime's in displayed text

Description

Creates a config point for specifying long / short date time formats. Defaults to humanized as we have today.

Provides for adding strftime formats into config.nu such as:

  datetime_format: {
    normal: "%Y-%m-%d %H:%M:%S"
    table: "%Y-%m-%d"
  }

Example:

> $env.config.datetime_format                                                                                                                         
┏━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ normal ┃ %a, %d %b %Y %H:%M:%S %z ┃
┃ table  ┃ %m/%d/%y %I:%M:%S%p      ┃
┗━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━┛
> let a = (date now)                                                                                                                                  
> echo $a                                                                                                                                             
Thu, 22 Jun 2023 10:21:23 -0700
> echo [$a]                                                                                                                                           
┏━━━┳━━━━━━━━━━━━━━━━━━━━━┓
┃ 0 ┃ 06/22/23 10:21:23AM ┃
┗━━━┻━━━━━━━━━━━━━━━━━━━━━┛

User-Facing Changes

Any place converting a datetime to a user displayed value should be impacted.

Tests + Formatting

  • cargo fmt --all -- --check Done
  • cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect -A clippy::result_large_err Done
  • cargo test --workspace Done
  • cargo run -- crates/nu-std/tests/run.nu Not done - doesn't seem to work
> use toolkit.nu  # or use an `env_change` hook to activate it automatically
> toolkit check pr
``` - Done

@fnordpig
Copy link
Contributor Author

I don't really understand what test is failing above - looks like some sort of virtualenv test?

@fdncred
Copy link
Collaborator

fdncred commented Jun 21, 2023

I don't really understand what test is failing above - looks like some sort of virtualenv test?

The venv issue is not related to your PR. It's crashing everyone's PR. We're trying to look into it.

@fdncred
Copy link
Collaborator

fdncred commented Jun 22, 2023

If we move forward with this PR we'll need the config point in the default_config.nu file with comments that help explain how to set the values.

I put this in my config. Is this appropriate?

  datetime_format: {
    normal: "%Y-%m-%d %H:%M:%S"
    abbreviated: "%Y-%m-%d"
  }

I'm not sure where normal and abbreviated show up in nushell. Any ideas?

@fnordpig
Copy link
Contributor Author

If we move forward with this PR we'll need the config point in the default_config.nu file with comments that help explain how to set the values.

I put this in my config. Is this appropriate?

  datetime_format: {
    normal: "%Y-%m-%d %H:%M:%S"
    abbreviated: "%Y-%m-%d"
  }

Yep that's precisely right! I'll add the default_config.nu changes to this PR

I'm not sure where normal and abbreviated show up in nushell. Any ideas?

Normal shows up wherever value.into_string/nonerror_into_string are called - it's a bit of a nest, because value::record/lazyrecord/list also defer to these types so it's tough for me to understand everywhere it shows up. I wish I had a better answer than that...

@fnordpig
Copy link
Contributor Author

fnordpig commented Jun 22, 2023

I'm not sure where normal and abbreviated show up in nushell. Any ideas?

Normal shows up wherever value.into_string/nonerror_into_string are called - it's a bit of a nest, because value::record/lazyrecord/list also defer to these types so it's tough for me to understand everywhere it shows up. I wish I had a better answer than that...

So, normal shows up if you do something like:

let a = (date now)
echo $a

Abbreviated appears to be generally when rendering a table that tries to fit on the screen, such as:

let a = [(date now)]
echo $a

@fnordpig
Copy link
Contributor Author

Added default_config.nu

@sholderbach
Copy link
Member

I am gonna be the pedantic party pooper for just a moment:

Can you update your PR description and title to make clear what changes and how one can use it, as soon as your design is ready for review. Both to help folks that want to play with it before it gets merged and also to have a good record in the commit history. Furthermore good examples in the PR descriptions make the life so much easier when we have to compile the release notes.

@fdncred
Copy link
Collaborator

fdncred commented Jun 22, 2023

Thanks for adding the default_config.nu!

I have one more request, instead of "normal" and "abbreviated" can we have "normal" and "ls" (or maybe "table")? I understand where abbreviated came from but I think it would be more intuitive for future users to intuitively understand which one shows up in ls.

@amtoine amtoine linked an issue Jun 22, 2023 that may be closed by this pull request
@amtoine
Copy link
Member

amtoine commented Jun 22, 2023

i think this should solve #8549, can you confirm that @fnordpig? 😋

Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to wrap this up by a review 😌

i love the feature, it looks great thanks @fnordpig 🥳

i agree with the remarks of @fdncred and @sholderbach, and i'll approve this with great pleasure once

@fnordpig
Copy link
Contributor Author

fnordpig commented Jun 22, 2023

I can confirm, it does resolve this. Obviously you have to do something like this:

> let-env config = ($env.config | upsert datetime_format.table '%m/%d/%y') 
> [1649203200000000000,1649289600000000000] | into datetime 
┏━━━┳━━━━━━━━━━┓
┃ 0 ┃ 04/06/22 ┃
┃ 1 ┃ 04/07/22 ┃
┗━━━┻━━━━━━━━━━┛

But assuming that's sufficient, it does indeed work.

Copy link
Collaborator

@fdncred fdncred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes I requested. I'm ready.

@fdncred
Copy link
Collaborator

fdncred commented Jun 23, 2023

I went ahead and made one change to the default_config.nu. Since this is the config file that most people start nushell with, I thought it was appropriate to leave our default human readable datetime in place. So, I left the table: setting in there but commented it out for people to experiment. Hope that's ok.

This is what I changed.

  datetime_format: {
    normal: '%a, %d %b %Y %H:%M:%S %z'  # shows up in displays of variables or other datetime's outside of tables
    # table: '%m/%d/%y %I:%M:%S%p'        # generally shows up in tabular outputs such as ls. commenting this out will change it to the default human readable datetime format
  }

Also, I'm not sure the default is appropriate for locales that use day/month/year vs month/day/year.

@sholderbach sholderbach changed the title Datetime Add custom datetime format through strftime strings Jun 23, 2023
@fdncred
Copy link
Collaborator

fdncred commented Jun 23, 2023

sorry, i broke your PR by landing another PR. would you mind fixing the merge conflicts?

amtoine and others added 6 commits June 23, 2023 07:50
related to 
- nushell#9404

# Description
to support our cratification effort and moving non-1.0 commands outside
of the main focus, this PR
- creates a new `nu-cmd-base` crate to hold the common structs, traits
and functions used by all command-related crates
- to start the transition, moves the `input_handler` module from
`nu-command` to `nu-cmd-base`

# User-Facing Changes
```
$nothing
```

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- ⚫ `toolkit test`
- ⚫ `toolkit test stdlib`

# After Submitting
```
$nothing
```
# Description

make history.max_size bigger so it doesn't eat history

closes nushell#9493

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This PR cleans up the deprecated legacy config options that were
deprecated in nushell version 0.72.0. These are the config points that
were in the root of the config but also duplicated in the nested
structures. For instance `use_ls_colors` was in the root of the config
and also in the `ls.use_ls_colors` nested structure. This was originally
done to preserve backwards compatibility when nested structures were
introduced in the config file.

Here's a list of the legacy config points that were removed.

- `use_ls_colors` - previously replaced with `ls.use_ls_colors`
- `rm_always_trash` - previously replaced with `rm.always_trash`
- `history_file_format` - previously replaced with `history.file_format`
- `sync_history_on_enter` - previously replaced with
`history.sync_on_enter`
- `max_history_size` - previously replaced with `history.max_size`
- `quick_completions` - previously replaced with `completions.quick`
- `partial_completions` - previously replaced with `completions.partial`
- `max_external_completion_results` - previously replaced with
`completions.external.max_results`
- `completion_algorithm` - previously replaced with
`completions.algorithm`
- `case_sensitive_completions` - previously replaced with
`completions.case_sensitive`
- `enable_external_completion` - previously replaced with
`completions.external.enable`
- `external_completer` - previously replaced with
`completions.external.completer`
- `table_mode` - previously replaced with `table.mode`
- `table_index_mode` - previously replaced with `table.index_mode`
- `table_trim` - previously replaced with `table.trim`
- `show_clickable_links_in_ls` - previously replaced with
`ls.clickable_links`
- `cd_with_abbreviations` - previously replaced with `cd.abbreviations`
- `filesize_metric` - previously replaced with `filesize.metric`
- `filesize_format` - previously replaced with `filesize.format`
- `cursor_shape_vi_insert` - previously replaced with
`cursor_shape.vi_insert`
- `cursor_shape_vi_normal` - previously replaced with
`cursor_shape.vi_normal`
- `cursor_shape_emacs` - previously replaced with `cursor_shape.emacs`

Removes log_level from the config since it doesn't do anything any
longer. We moved log-level to a nushell parameter some time ago.

Renames history_isolation to isolation in the config.nu for consistency.

Fixes a couple bugs where values weren't being set in the "//
Reconstruct" sections (history_isolation, table_show_empty).

Reorganized/Moved things around a tiny bit and added a few comments.

history.histor_isolation is now history.isolation.
If anyone is still using the legacy config points, deprecated since
0.72.0 2022-11-29, their config will break.

<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This PR cleans up the deprecated legacy config options that were
deprecated in nushell version 0.72.0. These are the config points that
were in the root of the config but also duplicated in the nested
structures. For instance `use_ls_colors` was in the root of the config
and also in the `ls.use_ls_colors` nested structure. This was originally
done to preserve backwards compatibility when nested structures were
introduced in the config file.

Here's a list of the legacy config points that were removed.

- `use_ls_colors` - previously replaced with `ls.use_ls_colors`
- `rm_always_trash` - previously replaced with `rm.always_trash`
- `history_file_format` - previously replaced with `history.file_format`
- `sync_history_on_enter` - previously replaced with
`history.sync_on_enter`
- `max_history_size` - previously replaced with `history.max_size`
- `quick_completions` - previously replaced with `completions.quick`
- `partial_completions` - previously replaced with `completions.partial`
- `max_external_completion_results` - previously replaced with
`completions.external.max_results`
- `completion_algorithm` - previously replaced with
`completions.algorithm`
- `case_sensitive_completions` - previously replaced with
`completions.case_sensitive`
- `enable_external_completion` - previously replaced with
`completions.external.enable`
- `external_completer` - previously replaced with
`completions.external.completer`
- `table_mode` - previously replaced with `table.mode`
- `table_index_mode` - previously replaced with `table.index_mode`
- `table_trim` - previously replaced with `table.trim`
- `show_clickable_links_in_ls` - previously replaced with
`ls.clickable_links`
- `cd_with_abbreviations` - previously replaced with `cd.abbreviations`
- `filesize_metric` - previously replaced with `filesize.metric`
- `filesize_format` - previously replaced with `filesize.format`
- `cursor_shape_vi_insert` - previously replaced with
`cursor_shape.vi_insert`
- `cursor_shape_vi_normal` - previously replaced with
`cursor_shape.vi_normal`
- `cursor_shape_emacs` - previously replaced with `cursor_shape.emacs`

Removes log_level from the config since it doesn't do anything any
longer. We moved log-level to a nushell parameter some time ago.

Renames history_isolation to isolation in the config.nu for consistency.

Fixes a couple bugs where values weren't being set in the "//
Reconstruct" sections (history_isolation, table_show_empty).

Reorganized/Moved things around a tiny bit and added a few comments.

history.histor_isolation is now history.isolation.
If anyone is still using the legacy config points, deprecated since
0.72.0 2022-11-29, their config will break.

<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
@fnordpig
Copy link
Contributor Author

Gah I've gotten into some sort of git hell

@fdncred
Copy link
Collaborator

fdncred commented Jun 23, 2023

been there, done that. don't worry too much about the amount of commits as they are all squashed anyway.

@fnordpig
Copy link
Contributor Author

Ok I seem to have navigated git hell successfully

@fdncred
Copy link
Collaborator

fdncred commented Jun 23, 2023

nice work, just some small fmt-clippy items to deal with and it should be good.

@fnordpig
Copy link
Contributor Author

Doh, merge artifacts. Fixed.

@fdncred
Copy link
Collaborator

fdncred commented Jun 23, 2023

Thanks!

@fdncred fdncred merged commit 0c88848 into nushell:main Jun 23, 2023
17 of 20 checks passed
@amtoine
Copy link
Member

amtoine commented Jun 25, 2023

this is really quite cool, thanks @fnordpig 👌

amtoine added a commit to amtoine/dotfiles that referenced this pull request Jun 26, 2023
bobhy added a commit to bobhy/nushell that referenced this pull request Jun 30, 2023
- improves usability of datetime's in displayed text
-
# Description
Creates a config point for specifying long / short date time formats.
Defaults to humanized as we have today.

Provides for adding strftime formats into config.nu such as:
```nu
  datetime_format: {
    normal: "%Y-%m-%d %H:%M:%S"
    table: "%Y-%m-%d"
  }
```

Example:
```bash
> $env.config.datetime_format
┏━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ normal ┃ %a, %d %b %Y %H:%M:%S %z ┃
┃ table  ┃ %m/%d/%y %I:%M:%S%p      ┃
┗━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━┛
> let a = (date now)
> echo $a
Thu, 22 Jun 2023 10:21:23 -0700
> echo [$a]
┏━━━┳━━━━━━━━━━━━━━━━━━━━━┓
┃ 0 ┃ 06/22/23 10:21:23AM ┃
┗━━━┻━━━━━━━━━━━━━━━━━━━━━┛
```

# User-Facing Changes
Any place converting a datetime to a user displayed value should be
impacted.

# Tests + Formatting

- `cargo fmt --all -- --check` Done
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` Done
- `cargo test --workspace` Done
- `cargo run -- crates/nu-std/tests/run.nu` Not done - doesn't seem to
work

```bash
> use toolkit.nu  # or use an `env_change` hook to activate it automatically
> toolkit check pr
``` - Done

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
Co-authored-by: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
@fnordpig fnordpig deleted the datetime branch July 2, 2023 03:48
amtoine added a commit that referenced this pull request Jul 12, 2023
related to a comment in #9500
> `cargo run -- crates/nu-std/tests/run.nu` Not done - doesn't seem to
work

this is absolutely true because the command in the PR template was
obsolete...
i've also updated the commands in the `CONTRIBUTING` document of the
library 👍

cc/ @fnordpig
sholderbach added a commit to sholderbach/nushell that referenced this pull request Nov 2, 2023
This is not perfect if the string is already illegal.

See nushell#9500
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

into datetime only gets last part of date in output when on list/table
4 participants