8000 Implement `TIME_STYLE="+DATE" ls` · Issue #3625 · uutils/coreutils · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implement TIME_STYLE="+DATE" ls #3625

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.

Sign up for GitHub

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

Jump to bottom
Closed
sylvestre opened this issue Jun 13, 2022 · 4 comments · Fixed by #3988
Closed

Implement TIME_STYLE="+DATE" ls #3625

sylvestre opened this issue Jun 13, 2022 · 4 comments · Fixed by #3988
Labels
good first issue For newcomers! U - ls

Comments

@sylvestre
Copy link
Contributor

We already support TIME_STYLE as an environment variable but only with the 4 different values.
GNU allows the use of +date

For example:

$ touch exe
$ TIME_STYLE="+%Y__%S" ls exe -al
-rw-r--r-- 1 sylvestre sylvestre 0 2022__58 exe
$ TIME_STYLE="+%Y-%m-%d %H:%M:%S.%N %z" ls exe -al
-rw-r--r-- 1 sylvestre sylvestre 0 2022-06-13 21:34:58.728327312 +0200 exe
$ TIME_STYLE="+foobar" ls exe -al
-rw-r--r-- 1 sylvestre sylvestre 0 foobar exe

GNU supports the date format (like the date or touch commands)
https://github.com/uutils/coreutils/blob/main/src/uu/date/src/date.rs
https://github.com/uutils/coreutils/blob/main/src/uu/touch/src/touch.rs

Tests in:
https://github.com/coreutils/coreutils/blob/master/tests/ls/color-term.sh#L23=
@sylvestre sylvestre moved this to Todo in GNU Compatibility Aug 18, 2022
@dmatos2012
Copy link
Contributor

Hi. I would also like to try this issue. I have made some progress, and technically it works except for the clap validation, see pr 8000 oblem on a question I made to clap-repo.

Basically right now we have value_parser!, but not supporting +FORMAT. Thus, by supporting +FORMAT, it cannot match the string anymore, but rather a user defined string with the + key in front, thus making it a bit more difficult, and as far as I am concerned, requiring a custom logic in case parse fails.

In my question to clap, the maintainer suggested something, and I tried some stuff but it looks so far a bit beyond my capabilities, so if anyone has any other ideas, lets discuss :)

@tertsdiepraam
Copy link
Member

Hi! Sounds very promising! You could always remove the parsing from clap, accept any string and then manually parse the string value later. What argument is this about by the way? In the issue I only see an environment variable, which clap should not have anything to do with right?

@dmatos2012
Copy link
Contributor
dmatos2012 commented Sep 27, 2022 •
edited
Loading

Hi! Your initial suggestion seems like doable. The argument is --time-style which accepts(for now) .value_parser(["full-iso", "long-iso", "iso", "locale"]). However if I add there +FORMAT then its no longer accepted. If I remove completely validation it works fine, but then you dont get the nice clap help message such as: Possible values are: ["full-iso", "long-iso", "iso", "locale"] etc.

@tertsdiepraam
Copy link
Member

Yeah we indeed lose the message and we'd need some custom message instead, but I'm not sure that the message from clap would still be as nice with a custom value_parser.

@dmatos2012 dmatos2012 mentioned this issue Sep 28, 2022
Merged
@tertsdiepraam tertsdiepraam linked a pull request Sep 29, 2022 that will close this issue
ls: add support for +FORMAT in timestyle #3988
Merged
Repository owner moved this from Todo to Done in GNU Compatibility Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue For newcomers! U - ls
Projects
GNU Compatibility
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ls: add support for +FORMAT in timestyle dmatos2012/coreutils
3 participants
@sylvestre @tertsdiepraam @dmatos2012
0