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 metadata function #95

Merged
merged 5 commits into from
Jan 10, 2024
Merged

Add metadata function #95

merged 5 commits into from
Jan 10, 2024

Conversation

jackpot51
Copy link
Contributor

@jackpot51 jackpot51 commented Jan 6, 2024

Resolves #94, adds a metadata function to get the type of the item (file or folder) and the size (in bytes for files or entries for folders). This is implemented for Freedesktop platforms and Windows

@jackpot51 jackpot51 marked this pull request as ready for review January 7, 2024 03:17
@jackpot51
Copy link
Contributor Author

jackpot51 commented Jan 7, 2024

I've also added a stub to allow unix platforms that aren't yet supported by get_mount_points (like Redox) to at least work with the home trash. I can do that in a different PR if desired.

Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for all the work you put in to make this happen!

Particularly Windows looks like it wasn't easy to get there (and it feels like one has to read a book to do anything on this platform).

The biggest change request is probably around how metadata is represented, and I hope it makes sense to you as well.

src/freedesktop.rs Show resolved Hide resolved
src/freedesktop.rs Outdated Show resolved Hide resolved
src/freedesktop.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/windows.rs Outdated
pub fn metadata(item: &TrashItem) -> Result<TrashItemMetadata, Error> {
ensure_com_initialized();
unsafe {
let id_as_wide: Vec<u16> = item.id.encode_wide().chain(std::iter::once(0)).collect();
Copy link
Owner

Choose a reason for hiding this comment

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

I don't want to burden you, but I'd love it if there was a to_wide_path(&OsStr) -> Vec<u16> or something that replaces this code here and in 5 other places in this file.

src/windows.rs Outdated Show resolved Hide resolved
@jackpot51
Copy link
Contributor Author

Thanks for the review! I will work on addressing these tomorrow

@jackpot51
Copy link
Contributor Author

@Byron I addressed most of your comments, please take a look.

- introduce function to create a wide path
- remove redundant `is_dir` field, but provide utilities to figure out if it is a directory or not.
@Byron
Copy link
Owner

Byron commented Jan 10, 2024

Thanks a lot for addressing most of the changes and for contributing, it's appreciated.

I did the rest of the changes and will merge in a moment.

@Byron Byron merged commit aa8e504 into Byron:master Jan 10, 2024
4 checks passed
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 file size and type to TrashItem
2 participants