Skip to content

Add mtime column toggle#21

Merged
alexdotc merged 4 commits intomainfrom
mtime
Oct 22, 2025
Merged

Add mtime column toggle#21
alexdotc merged 4 commits intomainfrom
mtime

Conversation

@alexdotc
Copy link
Copy Markdown
Member

@alexdotc alexdotc commented Oct 14, 2025

Notes / possible changes

  1. Clippy is mad about a 9 argument function
  2. It's possible there's a much slicker way to handle mtime parsing that involves less type conversions
  3. Maybe we can/should just calculate the current time once at startup for the format string comparison?

@alexdotc alexdotc linked an issue Oct 14, 2025 that may be closed by this pull request
@alexdotc alexdotc marked this pull request as ready for review October 15, 2025 21:57
@alexdotc alexdotc requested a review from lgarrison October 15, 2025 21:57
@alexdotc
Copy link
Copy Markdown
Member Author

alexdotc commented Oct 15, 2025

@lgarrison any criticism welcome - this adds a togglable/sortable column in the format style of ls -l

Separately, ceph also has an rctime xattr that might be useful

Copy link
Copy Markdown
Member

@lgarrison lgarrison left a comment

Choose a reason for hiding this comment

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

Looks great! Have you explored the rctime xattr at all? I assume it's like a recursive mtime, which could potentially be more intuitive, since all the other attributes we display are recursive. On the other hand, some old ceph bug reports seem to suggest it's not reliable or at least not updated promptly.

Comment thread src/ui.rs Outdated
Comment thread src/navigation.rs Outdated
@alexdotc
Copy link
Copy Markdown
Member Author

alexdotc commented Oct 21, 2025

I went ahead and swapped mtime out for ctime. We could provide both, but it seems slightly overloaded for now; we can always add it back in. I added a note to --help but I'm not sure if that's the best way to expose things. The note will be easier to split up and this might make a bit more sense with flags to control default sort/toggle on startup (sort of mentioned in #11)

It's a bit wider than the scope of this branch, but I also swapped out stat size for xattr size and did a bit of refactoring on the xattr calls. We now do 6 getxattr() calls per directory inode, but at least this should take advantage of MDS caching, which is probably the bigger consideration. And hardcoding xattr sizes per #7 will cut the number of calls in half.

Comment thread src/fs.rs
@alexdotc alexdotc requested a review from lgarrison October 21, 2025 21:49
@alexdotc
Copy link
Copy Markdown
Member Author

@lgarrison If we merge this, I'll try and do this before we cut a release:

And hardcoding xattr sizes per #7 will cut the number of calls in half.

Copy link
Copy Markdown
Member

@lgarrison lgarrison left a comment

Choose a reason for hiding this comment

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

Looks good! I tested it locally and it works great.

Comment thread src/main.rs Outdated
@lgarrison lgarrison added this to the v1.0 milestone Oct 22, 2025
@alexdotc alexdotc merged commit 1be4b88 into main Oct 22, 2025
2 checks passed
@alexdotc alexdotc deleted the mtime branch October 22, 2025 16:04
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.

add mtime column

2 participants