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

Associate WalkDir lifetime with WalkDirOptions, sorter, and IntoIter #203

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dbarnett
Copy link

Narrows lifetime of callbacks for sort_by and sort_by_key so they no longer need to be 'static.

Fixes #202.

Narrows lifetime of callbacks for sort_by and sort_by_key so they no
longer need to be 'static.
@dbarnett
Copy link
Author

Not sure if this is the ideal fix, but it allows narrowing the 'static lifetime while passing all tests. Please LMK if there's a better approach (or some reason the 'static is actually necessary).

@@ -231,30 +231,25 @@ pub type Result<T> = ::std::result::Result<T, Error>;
/// Note that when following symbolic/soft links, loops are detected and an
/// error is reported.
#[derive(Debug)]
pub struct WalkDir {
opts: WalkDirOptions,
pub struct WalkDir<'slf> {
Copy link
Owner

Choose a reason for hiding this comment

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

This is a breaking change. And this should also reveal why the WalkDir type requires 'static: so that it isn't itself generic.

This also strikes me as a very odd use of lifetimes and I don't know what kinds of implications it will have on downstream APIs.

Copy link
Author

Choose a reason for hiding this comment

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

Heh, well most use of lifetimes feels strange to me still... I was hoping in the process of implementing the change and getting your input I'd come to understand whether/how the implications were problematic, if they didn't happen to be trivially close to something workable.

@dbarnett
Copy link
Author

K, so from your comments here and on the bug, it seems like it's Working As Intended and 'static is exactly the lifetime you want on that? If so I can drop this and follow up with clarifications back on #202.

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.

Does sort_by_key really need 'static lifetime?
2 participants