From 307bd9366dce552a038716231bc9e582477f7b70 Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Tue, 22 Jun 2021 11:12:15 +1000 Subject: [PATCH] ui: In TransactionScreen, set the current transaction and index in tsInit based on the previous RegisterScreen. Use the RegisterScreen logic for selecting the new transaction when we cannot find the existing one. This enables us to get rid of regenerateTransactions. There is now different behaviour in the transaction screen when the journal is reloaded and the transaction being viewed is no longer available, but I have not been able to find an example which exhibits this different behaviour. I think it is better to have consistent behaviour between the register screen and transaction screen when determining which to select. This corrects a bug where you had to reload twice to reset the valuation and cost flags, due to the elimination of regenerateTransactions. --- hledger-ui/Hledger/UI/RegisterScreen.hs | 14 ++---- hledger-ui/Hledger/UI/TransactionScreen.hs | 55 +++++++--------------- 2 files changed, 20 insertions(+), 49 deletions(-) diff --git a/hledger-ui/Hledger/UI/RegisterScreen.hs b/hledger-ui/Hledger/UI/RegisterScreen.hs index 1e0d2a25c..c6aeb398d 100644 --- a/hledger-ui/Hledger/UI/RegisterScreen.hs +++ b/hledger-ui/Hledger/UI/RegisterScreen.hs @@ -72,7 +72,7 @@ rsInit d reset ui@UIState{aopts=_uopts@UIOpts{cliopts_=CliOpts{reportspec_=rspec -- always show historical balance -- , balancetype_= HistoricalBalance } - rspec' = + rspec' = either (error "rsInit: adjusting the query for register, should not have failed") id $ -- PARTIAL: updateReportSpec ropts' rspec @@ -365,17 +365,9 @@ rsHandle ui@UIState{ VtyEvent (EvKey (KChar 'z') [MCtrl]) -> suspend ui -- enter transaction screen for selected transaction - VtyEvent e | e `elem` moveRightEvents -> do + VtyEvent e | e `elem` moveRightEvents -> case listSelectedElement rsList of - Just (_, RegisterScreenItem{rsItemTransaction=t}) -> - let - ts = map rsItemTransaction $ V.toList $ nonblanks - numberedts = zip [1..] ts - i = maybe 0 (toInteger . (+1)) $ elemIndex t ts -- XXX - in - continue $ screenEnter d transactionScreen{tsTransaction=(i,t) - ,tsTransactions=numberedts - ,tsAccount=rsAccount} ui + Just _ -> continue $ screenEnter d transactionScreen{tsAccount=rsAccount} ui Nothing -> continue ui -- prevent moving down over blank padding items; diff --git a/hledger-ui/Hledger/UI/TransactionScreen.hs b/hledger-ui/Hledger/UI/TransactionScreen.hs index 0a71e43d6..8995dd06a 100644 --- a/hledger-ui/Hledger/UI/TransactionScreen.hs +++ b/hledger-ui/Hledger/UI/TransactionScreen.hs @@ -16,9 +16,11 @@ import Data.List import Data.Maybe import qualified Data.Text as T import Data.Time.Calendar (Day) +import qualified Data.Vector as V import Graphics.Vty (Event(..),Key(..),Modifier(..)) +import Lens.Micro ((^.)) import Brick -import Brick.Widgets.List (listMoveTo) +import Brick.Widgets.List (listElementsL, listMoveTo, listSelectedElement) import Hledger import Hledger.Cli hiding (progname,prognameandversion) @@ -43,16 +45,20 @@ transactionScreen = TransactionScreen{ tsInit :: Day -> Bool -> UIState -> UIState tsInit _d _reset ui@UIState{aopts=UIOpts{cliopts_=CliOpts{reportspec_=_rspec}} ,ajournal=_j - ,aScreen=TransactionScreen{} + ,aScreen=s@TransactionScreen{tsTransaction=(_,t),tsTransactions=nts} + ,aPrevScreens=prevscreens } = - -- plog ("initialising TransactionScreen, value_ is " - -- -- ++ (pshow (Just (AtDefault Nothing)::Maybe ValuationType)) - -- ++(pshow (value_ _ropts)) -- XXX calling value_ here causes plog to fail with: debug.log: openFile: resource busy (file is locked) - -- ++ "?" - -- ++" and first commodity is") - -- (acommodity$head$amounts$pamount$head$tpostings$snd$tsTransaction) - -- `seq` - ui + ui{aScreen=s{tsTransaction=(i',t'),tsTransactions=nts'}} + where + i' = maybe 0 (toInteger . (+1)) . elemIndex t' $ map snd nts' + -- If the previous screen was RegisterScreen, use the listed and selected items as + -- the transactions. Otherwise, use the provided transaction and list. + (t',nts') = case prevscreens of + RegisterScreen{rsList=xs}:_ -> (seltxn, zip [1..] $ map rsItemTransaction nonblanks) + where + seltxn = maybe nulltransaction (rsItemTransaction . snd) $ listSelectedElement xs + nonblanks = V.toList . V.takeWhile (not . T.null . rsItemDate) $ xs ^. listElementsL + _ -> (t, nts) tsInit _ _ _ = error "init function called with wrong screen type, should not happen" -- PARTIAL: tsDraw :: UIState -> [Widget Name] @@ -129,9 +135,8 @@ tsDraw _ = error "draw function called with wrong screen type, should not happen tsHandle :: UIState -> BrickEvent Name AppEvent -> EventM Name (Next UIState) tsHandle ui@UIState{aScreen=s@TransactionScreen{tsTransaction=(i,t) ,tsTransactions=nts - ,tsAccount=acct } - ,aopts=UIOpts{cliopts_=copts@CliOpts{reportspec_=rspec}} + ,aopts=UIOpts{cliopts_=copts} ,ajournal=j ,aMode=mode } @@ -171,7 +176,6 @@ tsHandle ui@UIState{aScreen=s@TransactionScreen{tsTransaction=(i,t) Right j' -> do continue $ regenerateScreens j' d $ - regenerateTransactions rspec j' s acct i $ -- added (inline) 201512 (why ?) clearCostValue $ ui VtyEvent (EvKey (KChar 'I') []) -> continue $ uiCheckBalanceAssertions d (toggleIgnoreBalanceAssertions ui) @@ -184,12 +188,10 @@ tsHandle ui@UIState{aScreen=s@TransactionScreen{tsTransaction=(i,t) VtyEvent (EvKey (KChar 'B') []) -> continue $ regenerateScreens j d $ - -- regenerateTransactions ropts d j s acct i $ toggleCost ui VtyEvent (EvKey (KChar 'V') []) -> continue $ regenerateScreens j d $ - -- regenerateTransactions ropts d j s acct i $ toggleValue ui VtyEvent e | e `elem` moveUpEvents -> continue $ regenerateScreens j d ui{aScreen=s{tsTransaction=(iprev,tprev)}} @@ -204,29 +206,6 @@ tsHandle ui@UIState{aScreen=s@TransactionScreen{tsTransaction=(i,t) tsHandle _ _ = error "event handler called with wrong screen type, should not happen" -- PARTIAL: --- Got to redo the register screen's transactions report, to get the latest transactions list for this screen. --- XXX Duplicates rsInit. Why do we have to do this as well as regenerateScreens ? -regenerateTransactions :: ReportSpec -> Journal -> Screen -> AccountName -> Integer -> UIState -> UIState -regenerateTransactions rspec j s acct i ui = - let - q = filterQuery (not . queryIsDepth) $ rsQuery rspec - thisacctq = Acct $ accountNameToAccountRegex acct -- includes subs - items = reverse $ accountTransactionsReport rspec j q thisacctq - ts = map first6 items - numberedts = zip [1..] ts - -- select the best current transaction from the new list - -- stay at the same index if possible, or if we are now past the end, select the last, otherwise select the first - (i',t') = case lookup i numberedts - of Just t'' -> (i,t'') - Nothing | null numberedts -> (0,nulltransaction) - | i > fst (last numberedts) -> last numberedts - | otherwise -> head numberedts - in - ui{aScreen=s{tsTransaction=(i',t') - ,tsTransactions=numberedts - ,tsAccount=acct - }} - -- | Select the nth item on the register screen. rsSelect i scr@RegisterScreen{..} = scr{rsList=l'} where l' = listMoveTo (i-1) rsList