refactor Actions to use MonadState

[frasertweedale]

"Make the whole country seethe with a high-pitched campaign for producing greenhouse vegetables!"

f87f82e (Fraser Tweedale, 33 minutes ago)
   use -XTypeApplications for 'focus' in keybindings

   Wanting to relax the return type of the 'focus' action from
   'AppState' to '()' revealed an opportunity to improve usability.  In 
   keybindings, when 'focus' is used we currently use an explicit, fully
   saturated type signature to determine the Focusable instance. This includes
   the 'Action' constructor and the 'AppState' return type, which are both
   redundant (`chain` and `chain'` both ignore the Action return type as a
   result of the StateT refactor).

   Therefore we can simplify the definition of keybindings that use
   'focus' by using -XTypeApplications, available in GHC since 8.0.

654f2c8 (Fraser Tweedale, 54 minutes ago)
   refactor invokeEditor' and editAttachment

   Refactor these functions to use 'MonadState'.  In the case of
   'editAttachment', a lot of the behaviour was subsumed by an application of
   'selectedItemHelper'.

e58e41d (Fraser Tweedale, 75 minutes ago)
   fix instance Focusable 'Threads 'ComposeFrom

   The old implementation (which was factored out to a top-level function
   'focusComposeFrom' contained the expression:

     if nullOf (asCompose . cAttachments) s
        then ...

   This was erroneously refacted as:

     l <- use (asCompose . cAttachments)
    if null l
      then ...

   The latter is testing whether the attachment list is empty, whereas the
   former is testing whether the optic (asCompose . cAttachments) has zero
   targets.  In the case of an Iso, Lens or Getter, there is always exactly
   one target, hence 'nullOf' always returns 'False'! The true branch of the
   old implementation is unreachable, but I introduced a bug (that made it
   reachable) during the refactor.

   To fix, remove the conditional and replace the whole conditional with the
   false branch.

c278f3d (Fraser Tweedale, 75 minutes ago)
   refactor Action to use StateT

   Using 'StateT AppState (EventM n)' for 'Action' definitions improves the
   readability (and writeability) of actions.  It also increases the
   flexibility (or abstractness) of what data types actions return.

   To get all this to work we had to add some orphan 'MonadThrow',
   'MonadCatch' and 'MonadMask' instances for '(EventM n)'.  I hope to land
   these upstream in brick.

[romanofski]

Merge once CI is green!

[frasertweedale]

@romanofski do you know what is going on with the nix job?

[romanofski]

One of the haddock strings is cactus:

src/UI/Actions.hs:1156:9: error:
    parse error on input ‘-- | Construct the full path to the attachment. The file browser only
        -- lists the file names (otherwise we wouldn't have the ability to
        -- display the full paths for deeper file hirarchies). However when
        -- attaching the file, we need the full paths so we can find, edit and
        -- update the attachments later.’
     |
1156 |         -- | Construct the full path to the attachment. The file browser only
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

[romanofski]

I'll fix it ...

[frasertweedale]

@romanofski don't we run haddock in the other builds too? Is there a different version of haddock on nix?

[romanofski]

Heh actually we do and they do fail: https://travis-ci.org/purebred-mua/purebred/jobs/647339453?utm_medium=notification&utm_source=github_status (scroll all the way down). The exit status must be swallowed tho ...

[frasertweedale]

@romanofski anyhow thanks for the fixup!

My brick PR to add MonadMask instance for EventM got merged already, so as soon as that's released we can bump with lower bound and drop the orphan instances. But that can happen later.

[frasertweedale]

@romanofski ok, that's a CI bug then. I'll file it.

[romanofski]

There also seems to be a version difference because this one: https://travis-ci.org/purebred-mua/purebred/jobs/647519007?utm_medium=notification&utm_source=github_status has a parse error in Type.hs, while the other builds seem fine.

[frasertweedale]

@romanofski if you're happy, merge it. I've got to head out shortly and won't be online all day :)

[romanofski]

Yeah lets merge. CI says green. For the non-green issues we can look separately.