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

Add lsdepth, findtree, cmin, and cmax #300

Merged
merged 3 commits into from
Apr 13, 2018

Conversation

cfhammill
Copy link
Contributor

As discussed in #298

Only issue here is that cmin and cmax need to be done in a MonadIO, so as far as I can tell it can't be used with mfilter with them directly.

Code review would be appreciated, I'm still pretty new to haskell.

Copy link
Owner

@Gabriella439 Gabriella439 left a comment

Choose a reason for hiding this comment

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

Looks good! Just one minor style suggestion

I'm also not that concerned if cmin and cmax have to be in IO, because in the worst case a user can do:

example = do
    file <- findtree somePattern (ls ".")
    True <- cmin someTIme file
    return file

timestamp
-}
cmin :: MonadIO io => UTCTime -> FilePath -> io Bool
cmin t file =
Copy link
Owner

Choose a reason for hiding this comment

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

Minor stylistic suggestion: I'd suggest using a non-pointfree function:

cmin t file = adapt <$> lstat file
  where
    adapt x = posixSecondsToUTCTime (modificationTime x) > t

-}
cmax :: MonadIO io => UTCTime -> FilePath -> io Bool
cmax t file =
adapt <$> lstat file
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like Travis is failing on older versions of GHC due to Functor not being a super-class of MonadIO. You can fix that by using liftM or do notation

@cfhammill
Copy link
Contributor Author

Hopefully this is fixed now, the build failed due to the hackage outage.

@Gabriella439
Copy link
Owner

Yep, that fixed it! Thanks for doing this :)

@Gabriella439 Gabriella439 merged commit 88a01f4 into Gabriella439:master Apr 13, 2018
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