Fix watch flag in commands other than 'build' (#153)

With the recent changes we added more "build" actions inside other commands,
which implies also accepting --watch in these other commands (expected behaviour: 
on file change rerun the command that is being watched).

However, the watch was only piped to the build command, so only the build part would 
be rerun on change. This fixes that by running the whole action on file change instead.

Author: f-f

app/Main.hs

@@ -8,8 +8,9 @@ import qualified Paths_spago        as Pcli
 import qualified System.Environment as Env
 import qualified Turtle             as T
 
-import           Spago.Build        (ExtraArg (..), ModuleName (..), SourcePath (..),
-                                     TargetPath (..), WithMain (..), Watch (..), NoBuild (..))
+import           Spago.Build        (BuildOptions (..), ExtraArg (..), ModuleName (..),
+                                     NoBuild (..), SourcePath (..), TargetPath (..), Watch (..),
+                                     WithMain (..))
 import qualified Spago.Build
 import           Spago.Packages     (PackageName (..), PackagesFilter (..))
 import qualified Spago.Packages
@@ -36,9 +37,8 @@ data Command
   -- | Generate documentation for the project and its dependencies
   | Docs [SourcePath]
 
-  -- | Build the project paths src/ and test/
-  --   plus the specified source paths
-  | Build (Maybe Int) Watch [SourcePath] [ExtraArg]
+  -- | Build the project paths src/ and test/ plus the specified source paths
+  | Build BuildOptions
 
   -- | List available packages
   | ListPackages (Maybe PackagesFilter)
@@ -50,18 +50,18 @@ data Command
   | VerifySet (Maybe Int)
 
   -- | Test the project with some module, default Test.Main
-  | Test (Maybe ModuleName) (Maybe Int) Watch [SourcePath] [ExtraArg]
+  | Test (Maybe ModuleName) BuildOptions
 
   -- | Run the project with some module, default Main
-  | Run (Maybe ModuleName) (Maybe Int) Watch [SourcePath] [ExtraArg]
+  | Run (Maybe ModuleName) BuildOptions
 
   -- | Bundle the project, with optional main and target path arguments
   --   Builds the project before bundling
-  | Bundle (Maybe ModuleName) (Maybe TargetPath) NoBuild [SourcePath] [ExtraArg]
+  | Bundle (Maybe ModuleName) (Maybe TargetPath) NoBuild BuildOptions
 
   -- | Bundle a module into a CommonJS module
   --   Builds the project before bundling
-  | MakeModule (Maybe ModuleName) (Maybe TargetPath) NoBuild [SourcePath] [ExtraArg]
+  | MakeModule (Maybe ModuleName) (Maybe TargetPath) NoBuild BuildOptions
 
   -- | Upgrade the package-set to the latest release
   | PackageSetUpgrade
@@ -106,12 +106,12 @@ parser = projectCommands
     watch = do
       res <- watchBool
       pure $ case res of
-        True -> Watch
+        True  -> Watch
         False -> BuildOnce
     noBuild = do
       res <- noBuildBool
       pure $ case res of
-        True -> NoBuild
+        True  -> NoBuild
         False -> DoBuild
     mainModule  = T.optional (T.opt (Just . ModuleName) "main" 'm' "The main module to bundle")
     toTarget    = T.optional (T.opt (Just . TargetPath) "to" 't' "The target file path")
@@ -120,6 +120,7 @@ parser = projectCommands
     packageName = T.arg (Just . PackageName) "package" "Specify a package name. You can list them with `list-packages`"
     packageNames = T.many $ T.arg (Just . PackageName) "package" "Package name to add as dependency"
     passthroughArgs = T.many $ T.arg (Just . ExtraArg) " ..any `purs compile` option" "Options passed through to `purs compile`; use -- to separate"
+    buildOptions = BuildOptions <$> limitJobs <*> watch <*> sourcePaths <*> passthroughArgs
     packagesFilter =
       let wrap = \case
             "direct"     -> Just DirectDeps
@@ -147,7 +148,7 @@ parser = projectCommands
     build =
       ( "build"
       , "Install the dependencies and compile the current package"
-      , Build <$> limitJobs <*> watch <*> sourcePaths <*> passthroughArgs
+      , Build <$> buildOptions
       )
 
     repl =
@@ -159,25 +160,25 @@ parser = projectCommands
     test =
       ( "test"
       , "Test the project with some module, default Test.Main"
-      , Test <$> mainModule <*> limitJobs <*> watch <*> sourcePaths <*> passthroughArgs
+      , Test <$> mainModule <*> buildOptions
       )
-      
+
     run =
       ( "run"
       , "Runs the project with some module, default Main"
-      , Run <$> mainModule <*> limitJobs <*> watch <*> sourcePaths <*> passthroughArgs
+      , Run <$> mainModule <*> buildOptions
       )
 
     bundle =
       ( "bundle"
       , "Bundle the project, with optional main and target path arguments"
-      , Bundle <$> mainModule <*> toTarget <*> noBuild <*> sourcePaths <*> passthroughArgs
+      , Bundle <$> mainModule <*> toTarget <*> noBuild <*> buildOptions
       )
 
     makeModule =
       ( "make-module"
       , "Bundle a module into a CommonJS module"
-      , MakeModule <$> mainModule <*> toTarget <*> noBuild <*> sourcePaths <*> passthroughArgs
+      , MakeModule <$> mainModule <*> toTarget <*> noBuild <*> buildOptions
       )
 
     docs =
@@ -298,16 +299,14 @@ main = do
     VerifySet limitJobs                   -> Spago.Packages.verify limitJobs Nothing
     PackageSetUpgrade                     -> Spago.Packages.upgradePackageSet
     Freeze                                -> Spago.Packages.freeze
-    Build limitJobs watch paths pursArgs  -> Spago.Build.build limitJobs watch paths pursArgs
-    Test modName limitJobs watch paths pursArgs
-                                          -> Spago.Build.test modName limitJobs watch paths pursArgs
-    Run modName limitJobs watch paths pursArgs
-                                          -> Spago.Build.run modName limitJobs watch paths pursArgs
+    Build buildOptions                    -> Spago.Build.build buildOptions Nothing
+    Test modName buildOptions             -> Spago.Build.test modName buildOptions
+    Run modName buildOptions              -> Spago.Build.run modName buildOptions
     Repl paths pursArgs                   -> Spago.Build.repl paths pursArgs
-    Bundle modName tPath build paths pursArgs
-                                          -> Spago.Build.bundle WithMain modName tPath build paths pursArgs
-    MakeModule modName tPath build paths pursArgs
-                                          -> Spago.Build.makeModule modName tPath build paths pursArgs
+    Bundle modName tPath shouldBuild buildOptions
+                                          -> Spago.Build.bundle WithMain modName tPath shouldBuild buildOptions
+    MakeModule modName tPath shouldBuild buildOptions
+                                          -> Spago.Build.makeModule modName tPath shouldBuild buildOptions
     Docs sourcePaths                      -> Spago.Build.docs sourcePaths
     Version                               -> printVersion
     PscPackageLocalSetup force            -> PscPackage.localSetup force

app/Spago/Build.hs

@@ -8,6 +8,7 @@ module Spago.Build
   , docs
   , Watch (..)
   , NoBuild (..)
+  , BuildOptions (..)
   , Purs.ExtraArg (..)
   , Purs.ModuleName (..)
   , Purs.SourcePath (..)
@@ -37,6 +38,13 @@ data Watch = Watch | BuildOnce
 --   or skip it, in the case of 'bundle' and 'makeModule'.
 data NoBuild = NoBuild | DoBuild
 
+data BuildOptions = BuildOptions
+  { maybeLimit      :: Maybe Int
+  , shouldWatch     :: Watch
+  , sourcePaths     :: [Purs.SourcePath]
+  , passthroughArgs :: [Purs.ExtraArg]
+  }
+
 defaultSourcePaths :: [Purs.SourcePath]
 defaultSourcePaths =
   [ Purs.SourcePath "src/**/*.purs"
@@ -53,16 +61,20 @@ prepareBundleDefaults maybeModuleName maybeTargetPath = (moduleName, targetPath)
     targetPath = fromMaybe (Purs.TargetPath "index.js") maybeTargetPath
 
 
--- | Build the project with purs, passing through
---   the additional args in the list
-build :: Maybe Int -> Watch -> [Purs.SourcePath] -> [Purs.ExtraArg] -> IO ()
-build maybeLimit shouldWatch sourcePaths passthroughArgs = do
+-- | Build the project with purs, passing through additional args and
+--   eventually running some other action after the build
+build :: BuildOptions -> Maybe (IO ()) -> IO ()
+build BuildOptions{..} maybePostBuild = do
   config <- Config.ensureConfig
   deps <- Packages.getProjectDeps config
   Packages.fetchPackages maybeLimit deps
   let projectGlobs = defaultSourcePaths <> sourcePaths
       allGlobs = Packages.getGlobs deps <> projectGlobs
-      buildAction = Purs.compile allGlobs passthroughArgs
+      buildAction = do
+        Purs.compile allGlobs passthroughArgs
+        case maybePostBuild of
+          Just action -> action
+          Nothing     -> pure ()
   absoluteProjectGlobs <- traverse makeAbsolute $ Text.unpack . Purs.unSourcePath <$> projectGlobs
   case shouldWatch of
     BuildOnce -> buildAction
@@ -78,59 +90,60 @@ repl sourcePaths passthroughArgs = do
 
 -- | Test the project: compile and run "Test.Main"
 --   (or the provided module name) with node
-test :: Maybe Purs.ModuleName -> Maybe Int -> Watch -> [Purs.SourcePath] -> [Purs.ExtraArg] -> IO ()
+test :: Maybe Purs.ModuleName -> BuildOptions -> IO ()
 test = runWithNode (Purs.ModuleName "Test.Main") (Just "Tests succeeded.") "Tests failed: "
 
 -- | Run the project: compile and run "Main"
 --   (or the provided module name) with node
-run :: Maybe Purs.ModuleName -> Maybe Int -> Watch -> [Purs.SourcePath] -> [Purs.ExtraArg] -> IO ()
+run :: Maybe Purs.ModuleName -> BuildOptions -> IO ()
 run = runWithNode (Purs.ModuleName "Main") Nothing "Running failed, exit code: "
 
 -- | Run the project with node: compile and run with the provided ModuleName
 --   (or the default one if that's missing)
-runWithNode :: Purs.ModuleName
-            -> Maybe T.Text
-            -> T.Text
-            -> Maybe Purs.ModuleName
-            -> Maybe Int
-            -> Watch
-            -> [Purs.SourcePath]
-            -> [Purs.ExtraArg]
-            -> IO ()
-runWithNode defaultModuleName maybeSuccessMessage failureMessage maybeModuleName
-            maybeLimit shouldWatch paths passthroughArgs = do
-  build maybeLimit shouldWatch paths passthroughArgs
-  T.shell cmd T.empty >>= \case
-    T.ExitSuccess   -> fromMaybe (pure ()) (echo <$> maybeSuccessMessage)
-    T.ExitFailure n -> die $ failureMessage <> T.repr n
+runWithNode
+  :: Purs.ModuleName
+  -> Maybe T.Text
+  -> T.Text
+  -> Maybe Purs.ModuleName
+  -> BuildOptions
+  -> IO ()
+runWithNode defaultModuleName maybeSuccessMessage failureMessage maybeModuleName buildOpts = do
+  build buildOpts (Just nodeAction)
   where
     moduleName = fromMaybe defaultModuleName maybeModuleName
     cmd = "node -e \"require('./output/" <> Purs.unModuleName moduleName <> "').main()\""
+    nodeAction = do
+      T.shell cmd T.empty >>= \case
+        T.ExitSuccess   -> fromMaybe (pure ()) (echo <$> maybeSuccessMessage)
+        T.ExitFailure n -> die $ failureMessage <> T.repr n
 
   -- | Bundle the project to a js file
-bundle :: Purs.WithMain -> Maybe Purs.ModuleName -> Maybe Purs.TargetPath -> NoBuild -> [Purs.SourcePath] -> [Purs.ExtraArg] -> IO ()
-bundle withMain maybeModuleName maybeTargetPath noBuild paths passthroughArgs =
+bundle :: Purs.WithMain -> Maybe Purs.ModuleName -> Maybe Purs.TargetPath -> NoBuild -> BuildOptions -> IO ()
+bundle withMain maybeModuleName maybeTargetPath noBuild buildOpts =
   let (moduleName, targetPath) = prepareBundleDefaults maybeModuleName maybeTargetPath
-  in do
-    case noBuild of
-      DoBuild -> build Nothing BuildOnce paths passthroughArgs
-      NoBuild -> pure ()
-    Purs.bundle withMain moduleName targetPath
+      bundleAction = Purs.bundle withMain moduleName targetPath
+  in case noBuild of
+    DoBuild -> build buildOpts (Just bundleAction)
+    NoBuild -> bundleAction
 
 -- | Bundle into a CommonJS module
-makeModule :: Maybe Purs.ModuleName -> Maybe Purs.TargetPath -> NoBuild -> [Purs.SourcePath] -> [Purs.ExtraArg] -> IO ()
-makeModule maybeModuleName maybeTargetPath noBuild paths passthroughArgs = do
+makeModule :: Maybe Purs.ModuleName -> Maybe Purs.TargetPath -> NoBuild -> BuildOptions -> IO ()
+makeModule maybeModuleName maybeTargetPath noBuild buildOpts =
   let (moduleName, targetPath) = prepareBundleDefaults maybeModuleName maybeTargetPath
       jsExport = Text.unpack $ "\nmodule.exports = PS[\""<> Purs.unModuleName moduleName <> "\"];"
-  echo "Bundling first..."
-  bundle Purs.WithoutMain (Just moduleName) (Just targetPath) noBuild paths passthroughArgs
-  -- Here we append the CommonJS export line at the end of the bundle
-  try (T.with
-        (T.appendonly $ T.fromText $ Purs.unTargetPath targetPath)
-        ((flip hPutStrLn) jsExport))
-    >>= \case
-      Right _ -> echo $ "Make module succeeded and output file to " <> Purs.unTargetPath targetPath
-      Left (n :: SomeException) -> die $ "Make module failed: " <> T.repr n
+      bundleAction = do
+        echo "Bundling first..."
+        Purs.bundle Purs.WithoutMain moduleName targetPath
+        -- Here we append the CommonJS export line at the end of the bundle
+        try (T.with
+              (T.appendonly $ T.fromText $ Purs.unTargetPath targetPath)
+              ((flip hPutStrLn) jsExport))
+          >>= \case
+            Right _ -> echo $ "Make module succeeded and output file to " <> Purs.unTargetPath targetPath
+            Left (n :: SomeException) -> die $ "Make module failed: " <> T.repr n
+  in case noBuild of
+    DoBuild -> build buildOpts (Just bundleAction)
+    NoBuild -> bundleAction
 
 -- | Generate docs for the `sourcePaths`
 docs :: [Purs.SourcePath] -> IO ()

test/spago-test.py

@@ -150,8 +150,8 @@
 
 
 ## spago bundle
-shutil.rmtree("./output") ## Remove output to ensure bundle builds as well as bundles
 
+shutil.rmtree("./output") ## Remove output to ensure bundle builds as well as bundles
 expect_success(
     ['spago', 'bundle', '--to', 'bundle.js'],
     "Spago should bundle successfully"
@@ -163,10 +163,12 @@
 
 
 ## spago make-module
-shutil.rmtree("./output") ## Remove output to ensure bundle builds as well as bundles
 
+# Now we don't remove the output folder, but we pass the `--no-build`
+# flag to skip rebuilding (i.e. we are counting on the previous command to
+# have built stuff for us)
 expect_success(
-    ['spago', 'make-module', '--to', 'module.js'],
+    ['spago', 'make-module', '--to', 'module.js', '--no-build'],
     "Spago should successfully make a module"
 )