Skip to content

Dwalk filter text out #348

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

brockpalen
Copy link

adds a large number of filters (predicates) from dfind to dwalk

@adammoody
Copy link
Member

Thanks for submitting this, @brockpalen . I should be able to take a look early next week.

@adammoody
Copy link
Member

Thanks again @brockpalen . Sorry that it took so long to get to this!

The --text-output option looks good, though let's flip the name to be --output-text to keep it close to --output under alphabetical listings. And we should keep the --text option active for a while for people who may have that option encoded into their scripts. We can deprecate -text, print a warning that points to the new option name, and then drop it in a future release. To be consistent, we'll want to add that option to each of the tools.

Would you be up for doing that?

I think the filtering looks fine. We could run into an explosion of options as we talked about before, especially if we decide other tools would also benefit from built-in filtering. Until we figure out how to deal with that, we could bring this into dwalk if you want as hidden functionality (supported in the source code, but undocumented in the help output and man pages). That would give you something that works until we can figure out if we want to go with this or come up with a different but equivalent approach.

One idea we talked about... I feel like there is probably a role to create a tool like dpipe that would parse and process a chained command like:

dpipe dfind --amin 60 /path/to/walk | dwalk --sort --output-text foo.txt

We'd probably want to pick something other than a | character so the user doesn't have to escape it in their shell command all of the time. To support this kind of structure easily, we'll need each tool to be implemented as a callable function. The tool function could take an input list (the pipe input) and produce an output list (the pipe output). This would be a lot of work, but it would let us keep a smaller number of options in each tool interface.

@brockpalen
Copy link
Author

Adam,

Sorry just getting back to this. I'm ok with what your propose. I will likely keep my own branch where I keep the help options. I have not put much effort into this because this gave me waht I wanted for my https://github.com/brockpalen/archivetar/ tool and is working quite well. Though it would be nice if there was a python module that could parse mfu file lists, and that's way beyond my copy and pasting your work.

I never did changes to a pull request before so hopefully hitting those resolve buttons worked. If not let me know, and if there is github documentation for what to do you can point me to it.

Copy link
Collaborator

@carbonneau1 carbonneau1 left a comment

Choose a reason for hiding this comment

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

You left some dead code that needs to be cleaned up. Once done, can you grab the latest and make sure to test with the latest.

thanks

-eric


mfu_pred* cur = p;
while (cur) {
// if (cur->f == MFU_PRED_PRINT || cur->f == MFU_PRED_EXEC) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you leave the dead code. Please, clean it up.

}

if (need_print) {
// mfu_pred_add(p, MFU_PRED_PRINT, NULL);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you leave the dead code. Please, clean it up.

@carbonneau1
Copy link
Collaborator

Could you integrate your changes with the latest code line? thanks

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.

3 participants