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

Added r_file_globsearch and zfs **.sig support #10793

Merged
merged 4 commits into from Jul 26, 2018

Conversation

@cyanpencil
Copy link
Contributor

@cyanpencil cyanpencil commented Jul 21, 2018

Closes #9690

I added dir.depth config var to restrict max depth in the recursive search. Default is 10, should be reasonable.

@cyanpencil cyanpencil force-pushed the cyanpencil:add-zfs-recursive branch 2 times, most recently from 12c45e5 to b3bcb5f Jul 23, 2018
@XVilka
Copy link
Contributor

@XVilka XVilka commented Jul 24, 2018

I don't think dir.depth is a good place for this option. dir.* namespace created solemnly to have various directories set. I would put it under search.* namespace.

@XVilka XVilka added this to the 2.8.0 milestone Jul 24, 2018
@XVilka XVilka added the RSearch label Jul 24, 2018
@cyanpencil
Copy link
Contributor Author

@cyanpencil cyanpencil commented Jul 24, 2018

Renamed dir.depth to search.dirdepth.

@radare
Copy link
Collaborator

@radare radare commented Jul 24, 2018

I dont think this option should be under search. Because its related to rutil. Not rsearch. I just wanted to have the renamed c api. Not the config var

@Maijin
Copy link
Contributor

@Maijin Maijin commented Jul 25, 2018

This has nothing to do under search indeed

@cyanpencil
Copy link
Contributor Author

@cyanpencil cyanpencil commented Jul 25, 2018

So, what do you suggest? @radare @Maijin what about zign.dirdepth?

@Maijin
Copy link
Contributor

@Maijin Maijin commented Jul 25, 2018

No because this can be used by more than zign

@Maijin
Copy link
Contributor

@Maijin Maijin commented Jul 25, 2018

I think dir. sounds the best for now

@XVilka
Copy link
Contributor

@XVilka XVilka commented Jul 26, 2018

Mmmm, okay, lets move it back to dir. even though I don't like its place then. Please rename it back and we'll merge.

@radare
Copy link
Collaborator

@radare radare commented Jul 26, 2018

@radare
Copy link
Collaborator

@radare radare commented Jul 26, 2018

@cyanpencil cyanpencil force-pushed the cyanpencil:add-zfs-recursive branch from 3c3f84c to e7f1f57 Jul 26, 2018
@cyanpencil
Copy link
Contributor Author

@cyanpencil cyanpencil commented Jul 26, 2018

Rebased and reverted back to dir.depth. Also fixed a small bug if the path started with \~

@Maijin
Copy link
Contributor

@Maijin Maijin commented Jul 26, 2018

Red in appveyor

@Maijin
Copy link
Contributor

@Maijin Maijin commented Jul 26, 2018

no red anymore 👍

@XVilka XVilka merged commit 6150481 into radareorg:master Jul 26, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants