b3nj5m1n / pfui

Efficiently generate content for statusbars, especially eww
MIT License
5 stars 1 forks source link

added disk monitoring #4

Open hardfau1t opened 1 year ago

b3nj5m1n commented 1 year ago

Hi, first of all, thank you for your continued interest in this project.

I'm having a couple of problems with this PR, nothing major, but I currently don't have the time to fix it myself. I could probably do it in the next few days, though.

My first problem is that MEDIA_DIR is created as a constant at compile time, including the USER environment variable. This means the binary will only work for the person who compiled it, and only on the same user. Also, this will fail to compile in certain environments. (nix, for example)

It seems that this constant is only used once, so it should be alright to just turn it into a variable in the function itself. I did this in my commit, so I hope you're okay with that solution.

My other problem is that the /media/run directory doesn't always exist, and when it doesn't, the program crashes, which it obviously shouldn't.

To address this, I think we'll have to turn mount_disk and drive_disk (or maybe just drive_disk) into Option<WatchDescriptor>. Since this might mean changes in a few more places, I don't want to do it at the moment, but as I said, if you don't want to fix this, I should be able to do it in a few days.

Overall, this is a great addition to the project, and thank you again for your contributions.

hardfau1t commented 1 year ago

To address this, I think we'll have to turn mount_disk and drive_disk (or maybe just drive_disk) into Option

problem with the making like this if the mount_dir /media/run is not present then we wont be able to monitor mounts. Another thing i forgot to mention is that this monitor is for udisks2 (on arch) this will grant user to mount as a non root user but drive will be mounted to /media/run directory.

if you don't want to fix this, I should be able to do it in a few days

sure I will see if I can provide any workaround for this, may be an option