Stop reformatting the config file at every run

README.md

@@ -324,17 +324,6 @@ $ spago run --node-args "arg1 arg2"
 ```
 
 
-### Avoid re-formatting the `spago.dhall` and `packages.dhall` with each command
-
-You can pass the `--no-config-format` or `-F` global flag:
-
-``` bash
-$ spago build -F
-Installation complete.
-Build succeeded.
-```
-
-
 ### Test my project
 
 You can also test your project with `spago`:

app/Curator.hs

@@ -408,7 +408,7 @@ withAST path transform = do
       newExpr <- transformMExpr transform expr
       echo $ "Done. Updating the \"" <> path <> "\" file.."
       writeTextFile (pathFromText path) $ Dhall.prettyWithHeader header newExpr <> "\n"
-      liftIO $ Dhall.format DoFormat path
+      liftIO $ Dhall.format path
   where
     transformMExpr
       :: Monad m

app/Spago.hs

@@ -151,7 +151,6 @@ parser = do
     depsOnly    = bool AllSources DepsOnly <$> CLI.switch "deps-only" 'd' "Only use sources from dependencies, skipping the project sources."
     clearScreen = bool NoClear DoClear <$> CLI.switch "clear-screen" 'l' "Clear the screen on rebuild (watch mode only)"
     noBuild     = bool DoBuild NoBuild <$> CLI.switch "no-build" 's' "Skip build step"
-    noFormat    = bool DoFormat NoFormat <$> CLI.switch "no-config-format" 'F' "Disable formatting the configuration file `spago.dhall`"
     jsonFlag    = bool JsonOutputNo JsonOutputYes <$> CLI.switch "json" 'j' "Produce JSON output"
     dryRun      = bool DryRun NoDryRun <$> CLI.switch "no-dry-run" 'f' "Actually perform side-effects (the default is to describe what would be done)"
     usePsa      = bool UsePsa NoPsa <$> CLI.switch "no-psa" 'P' "Don't build with `psa`, but use `purs`"
@@ -171,7 +170,7 @@ parser = do
     buildOptions  = BuildOptions <$> cacheFlag <*> watch <*> clearScreen <*> sourcePaths <*> noInstall <*> passthroughArgs <*> depsOnly
 
     -- Note: by default we limit concurrency to 20
-    globalOptions = GlobalOptions <$> verbose <*> noFormat <*> usePsa <*> fmap (fromMaybe 20) jobsLimit
+    globalOptions = GlobalOptions <$> verbose <*> usePsa <*> fmap (fromMaybe 20) jobsLimit
 
     projectCommands = CLI.subcommandGroup "Project commands:"
       [ initProject

src/Spago/Config.hs

@@ -30,12 +30,9 @@ import qualified Spago.Templates    as Templates
 import           Spago.PackageSet   (Package, PackageName, PackageSet)
 
 
-pathText :: Text
-pathText = "spago.dhall"
-
 -- | Path for the Spago Config
-path :: FilePath
-path = pathFromText pathText
+path :: IsString t => t
+path = "spago.dhall"
 
 
 -- | Spago configuration file type
@@ -106,8 +103,10 @@ parsePackage expr = die $ Messages.failedToParsePackage $ Dhall.pretty expr
 -- | Tries to read in a Spago Config
 parseConfig :: Spago m => m Config
 parseConfig = do
+  -- Here we try to migrate any config that is not in the latest format
   withConfigAST $ pure . addSourcePaths
-  expr <- liftIO $ Dhall.inputExpr $ "./" <> pathText
+
+  expr <- liftIO $ Dhall.inputExpr $ "./" <> path
   case expr of
     Dhall.RecordLit ks -> do
       packages :: Map PackageName Package <- Dhall.requireKey ks "packages" (\case
@@ -159,9 +158,9 @@ makeConfig :: Spago m => Bool -> m ()
 makeConfig force = do
   unless force $ do
     hasSpagoDhall <- testfile path
-    when hasSpagoDhall $ die $ Messages.foundExistingProject pathText
+    when hasSpagoDhall $ die $ Messages.foundExistingProject path
   writeTextFile path Templates.spagoDhall
-  Dhall.format DoFormat pathText
+  Dhall.format path
 
   -- We try to find an existing psc-package config, and we migrate the existing
   -- content if we found one, otherwise we copy the default template
@@ -243,13 +242,15 @@ filterDependencies expr = expr
 --   still be in the tree). If you need the resolved one, use `ensureConfig`.
 withConfigAST :: Spago m => (Expr -> m Expr) -> m ()
 withConfigAST transform = do
-  rawConfig <- liftIO $ Dhall.readRawExpr pathText
-  shouldFormat <- asks globalDoFormat
+  rawConfig <- liftIO $ Dhall.readRawExpr path
   case rawConfig of
     Nothing -> die Messages.cannotFindConfig
     Just (header, expr) -> do
       newExpr <- transformMExpr transform expr
-      liftIO $ Dhall.writeRawExpr shouldFormat pathText (header, newExpr)
+      -- Write the new expression only if it has actually changed
+      if (Dhall.Core.denote expr /= newExpr)
+        then liftIO $ Dhall.writeRawExpr path (header, newExpr)
+        else echoDebug "Transformed config is the same as the read one, not overwriting it"
 
 
 transformMExpr

src/Spago/Dhall.hs

@@ -22,12 +22,15 @@ type DhallExpr a = Dhall.Expr Parser.Src a
 
 
 -- | Format a Dhall file in ASCII
-format :: MonadIO m => DoFormat -> Text -> m ()
-format shouldFormat pathText =
-  when (shouldFormat == DoFormat) $
-    liftIO $ Dhall.Format.format
-      (Dhall.Format.Format Dhall.Pretty.ASCII $
-       Dhall.Format.Modify (Just $ Text.unpack pathText))
+--   We first check if it's already formatted, if not we reformat it.
+format :: MonadIO m => Text -> m ()
+format pathText = liftIO $
+  try (f $ Dhall.Format.Check path) >>= \case
+    Left (_e :: SomeException) -> f $ Dhall.Format.Modify path
+    Right _ -> pure ()
+  where
+    f = Dhall.Format.format . Dhall.Format.Format Dhall.Pretty.ASCII
+    path = Just $ Text.unpack pathText
 
 
 -- | Prettyprint a Dhall expression
@@ -53,14 +56,14 @@ readRawExpr pathText = do
     else (pure Nothing)
 
 
-writeRawExpr :: DoFormat -> Text -> (Text, DhallExpr Dhall.Import) -> IO ()
-writeRawExpr shouldFormat pathText (header, expr) = do
+writeRawExpr :: Text -> (Text, DhallExpr Dhall.Import) -> IO ()
+writeRawExpr pathText (header, expr) = do
   -- After modifying the expression, we have to check if it still typechecks
   -- if it doesn't we don't write to file.
   resolvedExpr <- Dhall.Import.load expr
-  _ <- throws (Dhall.TypeCheck.typeOf resolvedExpr)
+  throws (Dhall.TypeCheck.typeOf resolvedExpr)
   writeTextFile (pathFromText pathText) $ prettyWithHeader header expr <> "\n"
-  format shouldFormat pathText
+  format pathText
 
 
 -- | Returns a Dhall Text literal from a lone string

src/Spago/PackageSet.hs

@@ -5,7 +5,6 @@ module Spago.PackageSet
   , freeze
   , ensureFrozen
   , path
-  , pathText
   , PackageSet(..)
   , Package (..)
   , PackageLocation(..)
@@ -90,11 +89,8 @@ instance Dhall.Interpret Repo where
         Nothing   -> error $ Text.unpack $ Messages.failedToParseRepoString repo
 
 
-pathText :: Text
-pathText = "packages.dhall"
-
-path :: FilePath
-path = pathFromText pathText
+path :: IsString t => t
+path = "packages.dhall"
 
 
 -- | Tries to create the `packages.dhall` file if needed
@@ -103,8 +99,8 @@ makePackageSetFile force = do
   hasPackagesDhall <- testfile path
   if force || not hasPackagesDhall
     then writeTextFile path Templates.packagesDhall
-    else echo $ Messages.foundExistingProject pathText
-  Dhall.format DoFormat pathText
+    else echo $ Messages.foundExistingProject path
+  Dhall.format path
 
 
 -- | Tries to upgrade the Package-Sets release of the local package set.
@@ -122,7 +118,7 @@ upgradePackageSet = do
     Right releaseTagName -> do
       let quotedTag = surroundQuote releaseTagName
       echoDebug $ "Found the most recent tag for \"purescript/package-sets\": " <> quotedTag
-      rawPackageSet <- liftIO $ Dhall.readRawExpr pathText
+      rawPackageSet <- liftIO $ Dhall.readRawExpr path
       case rawPackageSet of
         Nothing -> die Messages.cannotFindPackages
         -- Skip the check if the tag is already the newest
@@ -134,7 +130,7 @@ upgradePackageSet = do
           echo $ "Upgrading the package set version to " <> quotedTag
           let newExpr = fmap (upgradeImports releaseTagName) expr
           echo $ Messages.upgradingPackageSet releaseTagName
-          liftIO $ Dhall.writeRawExpr DoFormat pathText (header, newExpr)
+          liftIO $ Dhall.writeRawExpr path (header, newExpr)
           -- If everything is fine, refreeze the imports
           freeze
   where
@@ -292,7 +288,7 @@ freeze = do
   echo Messages.freezePackageSet
   liftIO $
     Dhall.Freeze.freeze
-      (Just $ Text.unpack pathText)
+      (Just path)
       Dhall.Freeze.OnlyRemoteImports
       Dhall.Freeze.Secure
       Dhall.Pretty.ASCII
@@ -303,7 +299,7 @@ freeze = do
 ensureFrozen :: Spago m => m ()
 ensureFrozen = do
   echoDebug "Ensuring that the package set is frozen"
-  rawPackageSet <- liftIO $ Dhall.readRawExpr pathText
+  rawPackageSet <- liftIO $ Dhall.readRawExpr path
   case rawPackageSet of
     Nothing -> echo "WARNING: wasn't able to check if your package set file is frozen"
     Just (_header, expr) -> do

src/Spago/Prelude.hs

@@ -9,7 +9,6 @@ module Spago.Prelude
   , pathFromText
   , assertDirectory
   , GlobalOptions (..)
-  , DoFormat (..)
   , UsePsa(..)
   , Spago
   , module X
@@ -18,6 +17,7 @@ module Spago.Prelude
   , Text
   , NonEmpty (..)
   , Seq (..)
+  , IsString
   , Map
   , Generic
   , Alternative
@@ -96,6 +96,7 @@ import           Data.List.NonEmpty            (NonEmpty (..))
 import           Data.Map                      (Map)
 import           Data.Maybe                    as X
 import           Data.Sequence                 (Seq (..))
+import           Data.String                   (IsString)
 import           Data.Text                     (Text)
 import           Data.Text.Prettyprint.Doc     (Pretty)
 import           Data.Traversable              (for)
@@ -127,15 +128,11 @@ instance Show SpagoError where
   show (SpagoError err) = Text.unpack err
 
 
--- | Flag to skip automatic formatting of the Dhall files
-data DoFormat = DoFormat | NoFormat deriving (Eq)
-
 -- | Flag to disable the automatic use of `psa`
 data UsePsa = UsePsa | NoPsa
 
 data GlobalOptions = GlobalOptions
   { globalDebug    :: Bool
-  , globalDoFormat :: DoFormat
   , globalUsePsa   :: UsePsa
   , globalJobs     :: Int
   }

src/Spago/PscPackage.hs

@@ -84,7 +84,7 @@ insDhall = do
 
   PackageSet.ensureFrozen
 
-  try (dhallToJSON PackageSet.pathText packagesJsonPath) >>= \case
+  try (dhallToJSON PackageSet.path packagesJsonPath) >>= \case
     Right _ -> do
       echo $ "Wrote packages.json to " <> packagesJsonText
       echo "Now you can run `psc-package install`."

test/BumpVersionSpec.hs

@@ -3,11 +3,9 @@ module BumpVersionSpec (spec) where
 import           Prelude        hiding (FilePath)
 import qualified System.IO.Temp as Temp
 import           Test.Hspec     (Spec, around_, before_, describe, it, shouldBe)
-import           Turtle         (Text, cp, decodeString, mkdir, mv,
-                                 writeTextFile)
-import           Utils          (checkFileHasInfix, checkFixture, getHighestTag,
-                                 git, shouldBeFailure, shouldBeFailureInfix,
-                                 shouldBeSuccess, shouldBeSuccessInfix, spago,
+import           Turtle         (Text, cp, decodeString, mkdir, mv, writeTextFile)
+import           Utils          (checkFileHasInfix, checkFixture, getHighestTag, git,
+                                 shouldBeFailure, shouldBeFailureInfix, shouldBeSuccess, spago,
                                  withCwd)
 
 

test/PscPackageSpec.hs

@@ -2,10 +2,8 @@ module PscPackageSpec (spec) where
 
 import           Control.Monad.Extra (whenM)
 import           Prelude             hiding (FilePath)
-import           Test.Hspec          (Spec, afterAll_, around_, beforeAll,
-                                      describe, it, shouldBe)
-import           Turtle              (FilePath, empty, procStrictWithErr, procs,
-                                      rm, testdir, testfile)
+import           Test.Hspec          (Spec, afterAll_, around_, beforeAll, describe, it, shouldBe)
+import           Turtle              (FilePath, empty, procs, rm, testdir, testfile)
 import           Utils               (shouldBeSuccess, spago, withCwd)
 
 testDir :: FilePath

test/SpagoSpec.hs

@@ -1,13 +1,12 @@
 module SpagoSpec (spec) where
 
 import           Control.Concurrent (threadDelay)
-import           Data.Foldable      (for_)
 import qualified Data.Text          as Text
 import           Prelude            hiding (FilePath)
 import qualified System.IO.Temp     as Temp
 import           Test.Hspec         (Spec, around_, describe, it, shouldBe)
-import           Turtle             (ExitCode (..), cd, cp, decodeString, fromText, mkdir, mktree,
-                                     mv, readTextFile, rm, testdir, writeTextFile, shell, empty)
+import           Turtle             (ExitCode (..), cd, cp, decodeString, empty, mkdir, mktree, mv,
+                                     readTextFile, rm, shell, testdir, writeTextFile)
 import           Utils              (checkFixture, readFixture, runFor, shouldBeFailure,
                                      shouldBeFailureOutput, shouldBeSuccess, shouldBeSuccessOutput,
                                      spago, withCwd)

test/fixtures/run-no-psa.txt

@@ -1,6 +1,7 @@
 🍝
 Running NodeJS
 Running `spago build`
+Transformed config is the same as the read one, not overwriting it
 Ensuring that the package set is frozen
 Getting transitive deps
 Running `fetchPackages`

test/fixtures/run-output-psa-not-installed.txt

@@ -1,6 +1,7 @@
 🍝
 Running NodeJS
 Running `spago build`
+Transformed config is the same as the read one, not overwriting it
 Ensuring that the package set is frozen
 Getting transitive deps
 Running `fetchPackages`

test/fixtures/run-output.txt

@@ -1,6 +1,7 @@
 🍝
 Running NodeJS
 Running `spago build`
+Transformed config is the same as the read one, not overwriting it
 Ensuring that the package set is frozen
 Getting transitive deps
 Running `fetchPackages`