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

Improve getPagePath #15

Merged
merged 2 commits into from Nov 20, 2018
Merged

Improve getPagePath #15

merged 2 commits into from Nov 20, 2018

Conversation

ghost
Copy link

@ghost ghost commented Nov 20, 2018

The problem

The definition of Main.getPagePath is dependant on the length of Main.checkDirs:

getPagePath :: String -> IO (Maybe FilePath)
getPagePath page = do
  homeDir <- getHomeDirectory
  let pageDir = homeDir </> tldrDirName </> "tldr" </> "pages"
      x@(f1:f2:f3:f4:[]) = map (\x -> pageDir </> x </> page <.> "md") checkDirs
#if MIN_VERSION_base(4,7,0)
  f1' <- pageExists f1
  f2' <- pageExists f2
  f3' <- pageExists f3
  f4' <- pageExists f4
  return $ f1' <|> f2' <|> f3' <|> f4'
#else
  pageExists f1 <|> pageExists f2 <|> pageExists f3 <|> pageExists f4
#endif

This over complicates the process of adding new directories to Main.checkDirs.

The solution

Modify Main.getPagePath's definition to take advantage of Control.Monad's utilities. The result is:

getPagePath :: String -> IO (Maybe FilePath)
getPagePath page = do
  homeDir <- getHomeDirectory
  let pageDir = homeDir </> tldrDirName </> "tldr" </> "pages"
      paths = map (\x -> pageDir </> x </> page <.> "md") checkDirs
  foldr (<|>) Nothing <$> mapM pageExists paths

NOTE: I also disabled the CPP language extension, as, Main.getPagePath was the only function dependant CPP.

Status

This solution successfully compiles and runs on my Ubuntu 18.04.1 machine, with stack 1.7.1.

@psibi
Copy link
Owner

psibi commented Nov 20, 2018

@Kove-W-O-Salter Thanks! I think you can use foldr1 to make it more simplified ?

@ghost
Copy link
Author

ghost commented Nov 20, 2018

Yes, that would be very readable. The only thing is that it will fail if checkDirs is []. Do you want me to change it?

@psibi
Copy link
Owner

psibi commented Nov 20, 2018

Do you want me to change it?

Yes, checkDirs here will be always non empty.

@ghost
Copy link
Author

ghost commented Nov 20, 2018

Finished. I've changed the definition of getPagePath to:

getPagePath :: String -> IO (Maybe FilePath)
getPagePath page = do
  homeDir <- getHomeDirectory
  let pageDir = homeDir </> tldrDirName </> "tldr" </> "pages"
      paths = map (\x -> pageDir </> x </> page <.> "md") checkDirs
  foldr1 (<|>) <$> mapM pageExists paths

@psibi psibi merged commit 00a5a8a into psibi:master Nov 20, 2018
@psibi
Copy link
Owner

psibi commented Nov 20, 2018

Thanks!

@ghost
Copy link
Author

ghost commented Nov 20, 2018

Thank you 😄.

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.

1 participant