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

Fix -x option behavior #401

Merged
merged 4 commits into from
Jun 27, 2024
Merged

Conversation

rindeal
Copy link
Contributor

@rindeal rindeal commented May 27, 2024

This PR addresses an issue where target_dirs were being optimized out before being passed to the get_filesystem_devices() function. The changes in this PR ensure that target_dirs are passed directly to get_filesystem_devices(), and only then are they simplified.

Example of a command that currently doesn't work correctly:

dust -x $(findmnt -S tmpfs -o target -n)

It should show the usage of all tmpfs mounts, but it currently shows just the the root mountpoints like /run, since all the mountpoints nested under it are optimized out.

@rindeal rindeal changed the title do not optimize out target_dirs for get_filesystem_devices() Fix -x option behavior Jun 11, 2024
@bootandy
Copy link
Owner

Nice find, good edge case.

@bootandy
Copy link
Owner

Fix the lint and I'll accept - thanks

@rindeal rindeal force-pushed the get_filesystem_devices branch 6 times, most recently from 3540cea to 5b5ea73 Compare June 20, 2024 05:55
@rindeal
Copy link
Contributor Author

rindeal commented Jun 20, 2024

Done.

Also I streamlined the funcs so they all accept AsRef<Path>.

When running CI I noticed some tests are randomly failing, so I fixed that as well.

Hope you don't mind the new changes.

@rindeal rindeal force-pushed the get_filesystem_devices branch from 5b5ea73 to 4b82e1b Compare June 20, 2024 18:48
@bootandy
Copy link
Owner

Thanks,

Agree with the streamlining.

Those tests, were new tests and I refactored them here: #404 - so we no longer need that part of your PR. - which now causes a conflict :-(.

(Although I do like your use of $(mktemp -d) so feel free to keep that if you'd like).

rindeal added 4 commits June 25, 2024 04:29
This PR addresses an issue where `target_dirs` were being optimized out\
before being passed to the `get_filesystem_devices()` function.
The changes in this PR ensure that `target_dirs` are passed directly
to `get_filesystem_devices()`, and only then are they simplified.

Example of a command that currently doesn't work correctly:
```sh
dust -x $(findmnt -S tmpfs -o target -n)
```

It should show the usage of all tmpfs mounts, but it currently shows
just the the root mountpoints like `/run`, since all the mountpoints
nested under it are optimized out.
@rindeal rindeal force-pushed the get_filesystem_devices branch from 4b82e1b to 09b8de9 Compare June 25, 2024 04:35
@rindeal
Copy link
Contributor Author

rindeal commented Jun 25, 2024

Conflicts from those tests would be fine, but there were many complex conflicts from #399

mktemp -d would require needless code verbosity, your once initialization looked better to me, I just changed it to native code and to use a global constant to make it more robust.

@bootandy bootandy merged commit a06a001 into bootandy:master Jun 27, 2024
17 checks passed
@rindeal rindeal deleted the get_filesystem_devices branch June 27, 2024 22:46
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.

2 participants