psibi / tldr-hs

Haskell tldr client
BSD 3-Clause "New" or "Revised" License
92 stars 14 forks source link

Improve getPagePath #15

Closed ghost closed 5 years ago

ghost commented 5 years ago

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 commented 5 years ago

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

ghost commented 5 years ago

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 commented 5 years ago

Do you want me to change it?

Yes, checkDirs here will be always non empty.

ghost commented 5 years ago

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 commented 5 years ago

Thanks!

ghost commented 5 years ago

Thank you :smile:.