From 349ffd7c0b049f71a4071bf868ddacf7c11f2b1f Mon Sep 17 00:00:00 2001
From: Simon Michael <simon@joyful.com>
Date: Sat, 12 Dec 2020 12:05:44 -0800
Subject: [PATCH] updateReportSpecFromOpts -> updateReportSpec[With]

---
 hledger-lib/Hledger/Reports/ReportOptions.hs | 19 ++++++++++++-------
 hledger-ui/Hledger/UI/UIState.hs             | 10 +++++-----
 hledger/Hledger/Cli/Commands/Check.hs        |  8 ++++----
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/hledger-lib/Hledger/Reports/ReportOptions.hs b/hledger-lib/Hledger/Reports/ReportOptions.hs
index 5dde22496..988a7513f 100644
--- a/hledger-lib/Hledger/Reports/ReportOptions.hs
+++ b/hledger-lib/Hledger/Reports/ReportOptions.hs
@@ -18,7 +18,8 @@ module Hledger.Reports.ReportOptions (
   rawOptsToReportOpts,
   defreportspec,
   reportOptsToSpec,
-  updateReportSpecFromOpts,
+  updateReportSpec,
+  updateReportSpecWith,
   rawOptsToReportSpec,
   flat_,
   tree_,
@@ -245,12 +246,16 @@ reportOptsToSpec day ropts = do
       , rsQueryOpts = queryopts
       }
 
--- | Regenerate a ReportSpec after updating ReportOpts, or return an error
--- message if there is a problem such as missing or unparseable options data.
--- This helps keep the ReportSpec, its underlying ReportOpts, and the ReportOpts'
--- data fields like querystring_ all in sync.
-updateReportSpecFromOpts :: (ReportOpts -> ReportOpts) -> ReportSpec -> Either String ReportSpec
-updateReportSpecFromOpts f rspec = reportOptsToSpec (rsToday rspec) . f $ rsOpts rspec
+-- | Update the ReportOpts and the fields derived from it in a ReportSpec,
+-- or return an error message if there is a problem such as missing or 
+-- unparseable options data. This is the safe way to change a ReportSpec, 
+-- ensuring that all fields (rsQuery, rsOpts, querystring_, etc.) are in sync.
+updateReportSpec :: ReportOpts -> ReportSpec -> Either String ReportSpec
+updateReportSpec ropts rspec = reportOptsToSpec (rsToday rspec) ropts
+
+-- | Like updateReportSpec, but takes a ReportOpts-modifying function.
+updateReportSpecWith :: (ReportOpts -> ReportOpts) -> ReportSpec -> Either String ReportSpec
+updateReportSpecWith f rspec = reportOptsToSpec (rsToday rspec) . f $ rsOpts rspec
 
 -- | Generate a ReportSpec from RawOpts and the current date.
 rawOptsToReportSpec :: RawOpts -> IO ReportSpec
diff --git a/hledger-ui/Hledger/UI/UIState.hs b/hledger-ui/Hledger/UI/UIState.hs
index 0f4a4db2b..9a1c66410 100644
--- a/hledger-ui/Hledger/UI/UIState.hs
+++ b/hledger-ui/Hledger/UI/UIState.hs
@@ -55,7 +55,7 @@ uiShowStatus copts ss =
 reportSpecToggleStatusSomehow :: Status -> CliOpts -> ReportSpec -> ReportSpec
 reportSpecToggleStatusSomehow s copts =
     either (error "reportSpecToggleStatusSomehow: updating Status should not result in an error") id  -- PARTIAL:
-    . updateReportSpecFromOpts update
+    . updateReportSpecWith update
   where
     update = case maybeposintopt "status-toggles" $ rawopts_ copts of
       Just 2 -> reportOptsToggleStatus2 s
@@ -189,7 +189,7 @@ toggleReal ui@UIState{aopts=uopts@UIOpts{cliopts_=copts@CliOpts{reportspec_=rspe
     ui{aopts=uopts{cliopts_=copts{reportspec_=update rspec}}}
   where
     update = either (error "toggleReal: updating Real should not result in an error") id  -- PARTIAL:
-           . updateReportSpecFromOpts (\ropts -> ropts{real_=not $ real_ ropts})
+           . updateReportSpecWith (\ropts -> ropts{real_=not $ real_ ropts})
 
 -- | Toggle the ignoring of balance assertions.
 toggleIgnoreBalanceAssertions :: UIState -> UIState
@@ -237,14 +237,14 @@ updateReportPeriod updatePeriod ui@UIState{aopts=uopts@UIOpts{cliopts_=copts@Cli
     ui{aopts=uopts{cliopts_=copts{reportspec_=update rspec}}}
   where
     update = either (error "updateReportPeriod: updating period should not result in an error") id  -- PARTIAL:
-           . updateReportSpecFromOpts (\ropts -> ropts{period_=updatePeriod $ period_ ropts})
+           . updateReportSpecWith (\ropts -> ropts{period_=updatePeriod $ period_ ropts})
 
 -- | Apply a new filter query.
 setFilter :: String -> UIState -> UIState
 setFilter s ui@UIState{aopts=uopts@UIOpts{cliopts_=copts@CliOpts{reportspec_=rspec}}} =
     ui{aopts=uopts{cliopts_=copts{reportspec_=update rspec}}}
   where
-    update = either (const rspec) id . updateReportSpecFromOpts (\ropts -> ropts{querystring_=querystring})
+    update = either (const rspec) id . updateReportSpecWith (\ropts -> ropts{querystring_=querystring})
     querystring = words'' prefixes $ T.pack s
 
 -- | Reset some filters & toggles.
@@ -303,7 +303,7 @@ updateReportDepth updateDepth ui@UIState{aopts=uopts@UIOpts{cliopts_=copts@CliOp
     ui{aopts=uopts{cliopts_=copts{reportspec_=update rspec}}}
   where
     update = either (error "updateReportDepth: updating depth should not result in an error") id  -- PARTIAL:
-           . updateReportSpecFromOpts (\ropts -> ropts{depth_=updateDepth (depth_ ropts) >>= clipDepth ropts})
+           . updateReportSpecWith (\ropts -> ropts{depth_=updateDepth (depth_ ropts) >>= clipDepth ropts})
     clipDepth ropts d | d < 0            = depth_ ropts
                       | d >= maxDepth ui = Nothing
                       | otherwise        = Just d
diff --git a/hledger/Hledger/Cli/Commands/Check.hs b/hledger/Hledger/Cli/Commands/Check.hs
index 2332f0491..3a6a0a461 100644
--- a/hledger/Hledger/Cli/Commands/Check.hs
+++ b/hledger/Hledger/Cli/Commands/Check.hs
@@ -33,7 +33,7 @@ check copts@CliOpts{rawopts_} j = do
     args = listofstringopt "args" rawopts_
     -- reset the report spec that was generated by argsToCliOpts,
     -- since we are not using arguments as a query in the usual way
-    copts' = cliOptsUpdateReportSpec (\ropts -> ropts{querystring_=[]}) copts
+    copts' = cliOptsUpdateReportSpecWith (\ropts -> ropts{querystring_=[]}) copts
 
   case partitionEithers (map parseCheckArgument args) of
     (unknowns@(_:_), _) -> error' $ "These checks are unknown: "++unwords unknowns
@@ -81,8 +81,8 @@ runCheck copts@CliOpts{rawopts_} j (check,args) =
 -- underlying report options with the given update function.
 -- This can raise an error if there is a problem eg due to missing or
 -- unparseable options data. See also updateReportSpecFromOpts.
-cliOptsUpdateReportSpec :: (ReportOpts -> ReportOpts) -> CliOpts -> CliOpts
-cliOptsUpdateReportSpec roptsupdate copts@CliOpts{reportspec_} =
-  case updateReportSpecFromOpts roptsupdate reportspec_ of
+cliOptsUpdateReportSpecWith :: (ReportOpts -> ReportOpts) -> CliOpts -> CliOpts
+cliOptsUpdateReportSpecWith roptsupdate copts@CliOpts{reportspec_} =
+  case updateReportSpecWith roptsupdate reportspec_ of
     Left e   -> error' e  -- PARTIAL:
     Right rs -> copts{reportspec_=rs}