[GHC] #95: GHCi :edit command should jump to the the last error

[GHC]

#95: GHCi :edit command should jump to the the last error
-------------------------------+-------------------------------------------
        Reporter:              |            Owner:  lortabac
  martijnislief                |           Status:  patch
            Type:  feature     |        Milestone:  ⊥
  request                      |          Version:  None
        Priority:  normal      |         Keywords:
       Component:  GHCi        |     Architecture:  Unknown/Multiple
      Resolution:  None        |       Difficulty:  Easy (less than 1 hour)
Operating System:              |       Blocked By:
  Unknown/Multiple             |  Related Tickets:
 Type of failure:              |
  None/Unknown                 |
       Test Case:              |
        Blocking:              |
-------------------------------+-------------------------------------------

Comment (by nomeata):

 Better! So time for the next round of suggestions.

 I found this code confusing
 {{{
          let ...
                setGhciErrors e = writeIORef lastErrLocations (errs ++ e)
           setGhciErrors $ errLocation srcSpan
 }}}
 why not
 {{{
          let ...
          writeIORef lastErrLocations (errs ++ errLocation srcSpan)
 }}}
 In fact I find the whole section there a bit strange. Some suggestions:

  * Use `modifyIORef`
  * Write something like
    {{{
 case srcSpan of
   RealSrcSpan rsp -> modifyIORef lastErrLocations (++ [srcLocFile
 (realSrcSpanStart x) , srcLocLine (realSrcSpanStart x) ])
   _ -> return ()
    }}}
    so you don’t have to introduce so many local functions and do tricks
 with `++ []`.
  * `find (\(f, _) -> f == mkFastString file) errs` can even simplified to
 `lookup (mkFastSTring file)`

 I would put
 {{{
    ghciState <- lift getGHCiState
    liftIO $ writeIORef (lastErrorLocations ghciState) []
 }}}
 in a function `resetLastErrorLocations :: GHCi ()`. Naming that code also
 serves as documentation, and you don’t clutter the local name space of
 `doLoad` with the `ghciState`

 I wonder if things work well if the user uses a different path to the
 filename than GHC; maybe one wants to use a function that can check if two
 filenames point to the same file. But maybe that is a refinement that can
 be added later.

 Can you come up with a way to test this? Maybe by setting the editor to
 use to `/bin/echo`, which will put `+123` in the output that the test
 suite would check? See [Building/RunningTests/Adding] and shout if you
 need help creating a test case.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/95#comment:19>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler