Skip to content

Commit

Permalink
Simplify whichEith
Browse files Browse the repository at this point in the history
This commit simplifies the local `whichFull` function (local to `whichEith`)
by dropping a redundant guard as well as /always/ running
`lookupPath >>= leftPathError`. Previously we had this:

```
    whichFull fp = do
      (trace . mappend "which " . toTextIgnore) fp >> whichUntraced
      where
        whichUntraced | isAbsolute fp          = checkFile
                      | dotSlash splitOnDirs   = checkFile
                      | length splitOnDirs > 0 = lookupPath  >>= leftPathError
                      | otherwise              = lookupCache >>= leftPathError
        splitOnDirs = splitDirectories fp
```

However `splitOnDirs` can never return the empty list, so that guard was
redundant as that code path was always executed, and the /otherwise/
case never executed.
  • Loading branch information
adinapoli committed Apr 6, 2022
1 parent 51aff5e commit 238b5f4
Showing 1 changed file with 3 additions and 33 deletions.
36 changes: 3 additions & 33 deletions src/Shelly.hs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ import Control.Applicative
import Control.Concurrent
import Control.Concurrent.Async (async, wait, Async)
import Control.Exception
import Control.Monad ( when, unless, void, forM, filterM, liftM2 )
import Control.Monad ( when, unless, void, liftM2 )
import Control.Monad.Trans ( MonadIO )
import Control.Monad.Reader (ask)

Expand All @@ -122,22 +122,19 @@ import Data.Maybe
import Data.Semigroup ( (<>) )
#endif
import Data.Sequence ( Seq, (|>) )
import Data.Set ( Set )
import Data.Time.Clock ( getCurrentTime, diffUTCTime )
import Data.Tree ( Tree(..) )
import Data.Typeable

import qualified Data.ByteString as BS
import qualified Data.List as List
import qualified Data.Set as Set
import qualified Data.Text as T
import qualified Data.Text.IO as TIO
import qualified Data.Text.Encoding as TE
import qualified Data.Text.Encoding.Error as TE

import System.Directory
( setPermissions, getPermissions, Permissions(..), getTemporaryDirectory, pathIsSymbolicLink
, copyFile, removeFile, doesFileExist, doesDirectoryExist, listDirectory
, copyFile, removeFile, doesFileExist, doesDirectoryExist
, renameFile, renameDirectory, removeDirectoryRecursive, createDirectoryIfMissing
, getCurrentDirectory
)
Expand Down Expand Up @@ -607,8 +604,7 @@ whichEith originalFp = whichFull
where
whichUntraced | isAbsolute fp = checkFile
| startsWithDot splitOnDirs = checkFile
| length splitOnDirs > 0 = lookupPath >>= leftPathError
| otherwise = lookupCache >>= leftPathError
| otherwise = lookupPath >>= leftPathError

splitOnDirs = splitDirectories fp

Expand All @@ -631,7 +627,6 @@ whichEith originalFp = whichFull
-- function, but it returns \"./\" as its first argument,
-- so we pattern match on both for backward-compatibility.
startsWithDot (".":_) = True
startsWithDot ("./":_) = True
startsWithDot _ = False

checkFile :: Sh (Either String FilePath)
Expand All @@ -657,33 +652,8 @@ whichEith originalFp = whichFull
res <- liftIO $ isExecutable fullFp
return $ if res then Just fullFp else Nothing

lookupCache :: Sh (Maybe FilePath)
lookupCache = do
pathExecutables <- cachedPathExecutables
return $ fmap (flip (</>) fp . fst) $
List.find (Set.member fp . snd) pathExecutables


pathDirs = mapM absPath =<< ((map T.unpack . filter (not . T.null) . T.split (== searchPathSeparator)) `fmap` get_env_text "PATH")

cachedPathExecutables :: Sh [(FilePath, Set FilePath)]
cachedPathExecutables = do
mPathExecutables <- gets sPathExecutables
case mPathExecutables of
Just pExecutables -> return pExecutables
Nothing -> do
dirs <- pathDirs
executables <- forM dirs (\dir -> do
files <- (liftIO . listDirectory) dir `catch_sh` (\(_ :: IOError) -> return [])
exes <- fmap (map snd) $ liftIO $ filterM (isExecutable . fst) $
map (\f -> (f, takeFileName f)) files
return $ Set.fromList exes
)
let cachedExecutables = zip dirs executables
modify $ \x -> x { sPathExecutables = Just cachedExecutables }
return $ cachedExecutables


-- | A monadic findMap, taken from MissingM package
findMapM :: Monad m => (a -> m (Maybe b)) -> [a] -> m (Maybe b)
findMapM _ [] = return Nothing
Expand Down

0 comments on commit 238b5f4

Please sign in to comment.