Opened 17 years ago

Closed 6 years ago

Last modified 5 years ago

#95 closed feature request (fixed)

GHCi :edit command should jump to the the last error

Reported by: martijnislief Owned by: lortabac
Priority: normal Milestone:
Component: GHCi Version: None
Keywords: Cc: hvr
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by simonmar)

GHCi has a :edit command for editing the current source file. Hugs's command of the same name automatically jumps the editor to the location of the last error; it would be nice if GHCi's did the same.

Attachments (4)

le.ghci (1.3 KB) - added by claus 12 years ago.
example .ghci script implementing :le (load/reload and edit)
0001-Bug-95-edit-command-should-jump-to-the-last-error.patch (4.9 KB) - added by lortabac 6 years ago.
0001-Fixes-95-edit-command-should-jump-to-the-last-error.patch (7.6 KB) - added by lortabac 6 years ago.
0001-Fixes-95-edit-command-should-jump-to-the-last-error.2.patch (10.5 KB) - added by lortabac 6 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 13 years ago by simonmar

Architecture: Unknown
Description: modified (diff)
difficulty: Easy (1 hr)
Operating System: Unknown

I implemented :edit, but without the line-number-jumping facility for now.

comment:2 Changed 13 years ago by igloo

Component: NoneGHCi
Milestone: 6.6.1

comment:3 Changed 13 years ago by simonmar

Priority: normallow

comment:4 Changed 13 years ago by igloo

Priority: lownormal

Punting on this.

The only problem with fixing it is that we've forgotten where the first error was by the time :e is given; shouldn't be too hard to fix, though, by updating a mutable variable when appropriate.

comment:5 Changed 13 years ago by simonmar


comment:6 Changed 12 years ago by claus

starting somewhere in 6.7, you can define something like that yourself. the trick is to have a :redirErr command that allows you to capture ghci command error output. then you do a :l or :r, capture the errors, and use :e to edit the first one.

a slight complication is that :l/:r invalidate all variable bindings, so we need to define a temporary :afterCmd, in order to restore stderr and to read the errors after the :l/:r.

please note that many editors support quick compile/edit cycles (in vim, try :help quickfix; emacs should also support something similar), which will do this kind of thing more easily in most cases.

add the attached le.ghci to your .ghci, or source it with :cmd readFile "le.ghci". then, :le <mod> will :load and :edit, and :le on its own will :reload and :edit. adapt the code to your needs.

there is a patch pending for ghc head that will allow us to avoid having to compress ghci definitions into single lines, which will make things like this a lot more readable and maintainable..

Changed 12 years ago by claus

Attachment: le.ghci added

example .ghci script implementing :le (load/reload and edit)

comment:7 Changed 12 years ago by guest

actually, it turns out you can define all this even back in 6.4.1 (had i only known back then!-). you need to define your own :e command (:def is available), and ghci 6.4.1 is somewhat annoying about command names that are prefixes of each other. have a look at

since .ghci files are not very readable with definitions squashed into single lines, you might want to check the explanations of the latter file in

(note: the names in the 6.4.1 version differ from those in the described version)

i'm not yet sure whether being able to define :e means that it shouldn't be part of ghci at all (and this ticket be closed), or whether there are still any advantages to having it built in? perhaps ghci releases should have a selection of .ghci files with useful definitions?

comment:8 Changed 12 years ago by simonmar

Owner: nobody deleted
Status: assignednew

comment:9 Changed 12 years ago by igloo

Milestone: 6.8 branch6.10 branch

comment:10 Changed 11 years ago by simonmar

Architecture: UnknownUnknown/Multiple

comment:11 Changed 11 years ago by simonmar

Operating System: UnknownUnknown/Multiple

comment:12 Changed 11 years ago by igloo

Milestone: 6.10 branch6.12 branch

comment:13 Changed 10 years ago by simonmar

difficulty: Easy (1 hr)Easy (less than 1 hour)

comment:14 Changed 10 years ago by simonmar

Description: modified (diff)
Milestone: 6.12 branch_|_
Summary: GHCi editor binding with ":e"GHCi :edit command should jump to the the last error
Type of failure: None/Unknown

You can define it yourself, but it's a bit hacky and it would clearly be better to have it built-in. This is also a small self-contained task (limited to the GHCi front end) that a new contributor could take on.

comment:15 Changed 6 years ago by lortabac

Cc: hvr added
Status: newpatch

comment:16 Changed 6 years ago by nomeata

Hi lortabac. Thanks for your patch.

One thing that is definitely missing is documentation: You should explain this behaviour in the user manual for :edit.

I would name the field errors a bit more self-descriptive. Maybe lastErrorLocations?

Why do you need unsafeCoerce? Can’t you create the IORef locally when initializing GHCiState, in interactiveUI, and also pass it as a first parameter to your ghciLogAction?

Your filter could be more readable a find (from Data.List).

That’s it for now :-)

comment:17 Changed 6 years ago by nomeata

Owner: set to lortabac

comment:18 Changed 6 years ago by lortabac

I followed your suggestions and I also fixed a small mistake.

comment:19 Changed 6 years ago 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.

comment:20 Changed 6 years ago by lortabac

I added a sameFile function to check whether two paths point to the same file. However I can't find any readable alternative to:

    curFileErrs <- filterM (\(f, _) -> unpackFS f `sameFile` file) errs
    return $ case curFileErrs of
        (_, line):_ -> " +" ++ show line
        _ -> ""

As far as the test is concerned, I just followed the enumeration in the ghci directory, so I called it prog013. It would be nice to make it a bit more self-descriptive but I have no idea how.

comment:21 Changed 6 years ago by nomeata

Don’t worry about test names, prog013 is fine; T95 would have been another possibility. Ideally, the test case is never looked at again, because nobody breaks this feature.

The patch is, IMHO, good to go. I’m validating this right now and will push if validation succeeds.

comment:22 Changed 6 years ago by Joachim Breitner <mail@…>

In ce19d5079ea85d3190e837a1fc60000fbd82134d/ghc:

Fixes #95 :edit command should jump to the last error

comment:23 Changed 6 years ago by nomeata

Resolution: Nonefixed
Status: patchclosed

Thanks, Lorenzo, for your first contribution to GHC! I hope more will follow :-)

comment:24 Changed 5 years ago by Austin Seipp <austin@…>

In 4756438962a76d2dcedf63b90ec789cb054f9556/ghc:

Catch canonicalizePath exceptions, fix #10101

Introduce by #95 'canonicalizePath' throws and exception when given
an invalid file in a call to 'sameFile'.

There are two cases when this can happen when using ghci:
  1) If there is an error at the interactive prompt, "<interactive>"
     file is searched for and not found.
  2) If there is an error in any loaded file and editing an inexistent/new
     file with 'e: foo'.

Both cases are now tested.

Test Plan: validate

Reviewers: austin, #ghc

Reviewed By: austin, #ghc

Subscribers: bgamari, thomie

Differential Revision:

GHC Trac Issues: #10101
Note: See TracTickets for help on using tickets.