Skip to content

Improve getPagePath#15

Merged
psibi merged 2 commits intomasterfrom
unknown repository
Nov 20, 2018
Merged

Improve getPagePath#15
psibi merged 2 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner

psibi commented Nov 20, 2018

Do you want me to change it?

Yes, checkDirs here will be always non empty.

@ghost
Copy link
Copy Markdown
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
Copy Markdown
Owner

psibi commented Nov 20, 2018

Thanks!

@ghost
Copy link
Copy Markdown
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