Added ordinals support for time block #1824
Conversation
|
FYI, |
|
Thanks @MaxVerevkin, I test pre-commit locally then commit |
|
@virajbhartiya I believe you pushed to the wrong branch. |
|
Yes, I was actually working on the padding issue but forgot the switch branches. |
|
Hey @MaxVerevkin, can you help me understand why the checks are failing, I haven't made any changes in the cargo.toml file but still it's showing errors in it |
| @@ -43,6 +43,7 @@ smart-default = "0.6" | |||
| swayipc-async = "2.0" | |||
| tokio-stream = "0.1" | |||
| toml = "0.7" | |||
| tera = "1.13" | |||
There was a problem hiding this comment.
Why do you need tera?
| @@ -86,7 +88,13 @@ pub async fn run(config: Config, mut api: CommonApi) -> Result<()> { | |||
|
|
|||
| widget.set_values(map!( | |||
| "icon" => Value::icon(api.get_icon("time")?), | |||
| "timestamp" => Value::datetime(Utc::now(), timezone.copied()) | |||
| "timestamp" => { | |||
There was a problem hiding this comment.
Looks like this will be a regression of #1723
| let datetime: DateTime<Utc> = Utc::now(); | ||
| let format: Option<&str> = Some("%a %e{S} %B, %H:%M"); | ||
|
|
||
| Value::datetime(datetime, timezone, Some(format)) |
There was a problem hiding this comment.
The third parameter would be Option<Option<&str>>, which probably isn't what you want.
| @@ -104,11 +112,3 @@ pub async fn run(config: Config, mut api: CommonApi) -> Result<()> { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
@MaxVerevkin we need this to be able to get the timezone if the user change /etc/localtime, right?
There was a problem hiding this comment.
Yes. @virajbhartiya added use libc::tzset, but I can't find it in libc docs?
| @@ -81,6 +82,12 @@ pub fn new_formatter(name: &str, args: &[Arg]) -> Result<Box<dyn Formatter>> { | |||
| .error("Interval must be a positive number")?, | |||
| ); | |||
| } | |||
| "padding" => { | |||
There was a problem hiding this comment.
Looks like this code is part of the wrong PR.
|
Not really sure what is happening here, but just to make sure - the original issue was to add support for formatting the number of the day of the month with the appropriate suffix such as Also the format string for the time block already had a breaking change in the upgrade to v0.30, so we shouldn't make anymore breaking changes especially not for a niche feature such as this one. |
| //! full = " $icon $timestamp.datetime(f:'%a %Y-%m-%d %R %Z', l:fr_BE) " | ||
| //! short = " $icon $timestamp.datetime(f:%R) " | ||
| //! full = " $icon $timestamp.datetime(f:'%a %e{S} %B, %H:%M', l:Europe/Lisbon) " | ||
| //! short = " $icon $timestamp.datetime(f:%R, l:Europe/Lisbon) " |
There was a problem hiding this comment.
l: is for locale, not timezone
There was a problem hiding this comment.
Why are these changes even necessary?
Fixed #1323
I've added Ordinal support for time using
chrono