-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
More detailed StatStruct show #39463
Conversation
7634bb4
to
28dc1a7
Compare
We typically like to keep For the short form, we could mimic BSD:
For this full form, we could mimic Linux stat (BSD -x):
|
Yeah, nice. Do we have functions to print the mode and any others in their more readable forms? It'd be nice to also print the file path, but that's not part of StatStruct. Could we expand StatStruct to include it? |
For the moment, I'm just working on the way the fields are printed. The times now are shown in local time, with a human-friendly time difference in most significant units only, up to units of days
Two questions:
|
Now with:
|
I can work to fix the tests if we're ok adding a field, @vtjnash ? Also, I'm still planning to make a compact show the default, just working on getting the fields right still. Can any other fields be more richly printed? |
Now with username and group name lookup on Linux and MacOS
|
Updated with a compact single-line form as default
|
@vtjnash no rush on this, but do you think it's getting close? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this seems getting close
base/stat.jl
Outdated
@@ -26,6 +26,7 @@ export | |||
uperm | |||
|
|||
struct StatStruct | |||
desc :: Union{AbstractString, OS_HANDLE} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will change ==
and hash
, but I don't think people would have been relying on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@staticfloat just a thought.. is the StatStruct
hash ever used for artifact hashing etc.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GIven this broke existing symlink tests, I've defined custom hash and equality methods that skip desc
and added a note to the struct
base/stat.jl
Outdated
println(iob, " ctime: $(iso_datetime_with_relative(st.ctime, tnow))") | ||
end | ||
else | ||
sprint() do iob |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the code for a different function (instead of compact):
sprint() do iob | |
function show(io::IO, ::MIME"text/plain", st::StatStruct) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean that you wouldn't have the user select compact or not via the compact flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are passing now, and I believe this is the only outstanding issue, but I need help understanding what you mean. Are you suggesting the user selects the compact or full versions via some other mechanism than setting julia> Base.active_repl.options.iocontext[:compact] = false
?
Setting compact
via that method is rather laborious, so I'm definitely not attached to it.
bump? |
065d46e
to
511b655
Compare
I won't have more time for this for a while, so feel free to push to it if you want to push it over the line @vtjnash Edit: Just getting tests passing |
a6acc40
to
568a69a
Compare
I think we should just merge this. If noone else responds in the next day or two, I will. |
I think there were a number of minor issues I've been meaning to address, but just haven't gotten time |
Happy to give a stab at finishing this, but I need some clarification on the thread above at least |
Mostly finished. Will push when I'm done :) |
568a69a
to
1e97b0d
Compare
okay, I pushed a cleanup commit (squash when merging) |
@vtjnash is the cause of the error here obvious?
|
Bump @vtjnash |
1e97b0d
to
9fc7a41
Compare
oops, I miscounted the bytes, and didn't realize I had asserts disabled |
This fails on Windows: https://build.julialang.org/#/builders/66/builds/3795/steps/5/logs/stdio |
Need to check for the correct substring.
#ifdef _OS_WINDOWS_ | ||
return UV_ENOTSUP; | ||
#else | ||
// taken directly from libuv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call libuv instead of copying the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added an extra argument, so we need to upstream this first.
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Need to check for the correct substring.
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Need to check for the correct substring.
Perhaps the show for StatStruct could display more than it currently does?
Currently
This PR
Edit: latest version