#1363: Sourcing multi-line scripts in GHCi: track line numbers, and bail out
after
first error
---------------------------------+------------------------------------------
Reporter: Frederik | Owner: vivian
Type: feature request | Status: patch
Priority: normal | Milestone: _|_
Component: GHCi | Version: 6.6
Keywords: multiline script | Testcase:
Blockedby: | Difficulty: Moderate (less than a day)
Os: Unknown/Multiple | Blocking:
Architecture: Unknown/Multiple | Failure: None/Unknown
---------------------------------+------------------------------------------
Comment(by igloo):
Sorry for the delay in looking at this patch. I just had a look at it (or
rather, the one in http://www.haskell.org/pipermail/cvs-
ghc/2011-January/059156.html) and I've got a few comments; first a few
small things, then a couple of larger issues.
It would be good to have a test or two for `:script`, and also for the
filenames and line numbers in errors.
`sourceConfigFile` ought to set `progname` too.
Rather than using `all isDigit first` and `read first`, I think it'd be a
little nicer to use `reads`.
This looks like a typo (missing ">"):
{{{
+hscStmt hsc_env stmt = hscStmtWithLocation hsc_env stmt "<interactive" 1
}}}
but I couldn't work out when that string is printed. The fast testsuite
doesn't seem to break if I change it, so perhaps there is another codepath
we should be testing?
Rather than the
{{{
fileLoop script >>= incrementLines
}}}
idiom I think it would be better to make `fileLoop` always increment the
line number.
Code like this worries me:
{{{
+ st <- lift $ getGHCiState
+ let prog = progname st
+ line = line_number st
+ lift $ setGHCiState st{progname=filename,line_number=0}
+ scriptLoop script
+ liftIO $ hClose script
+ lift $ setGHCiState st{progname=prog,line_number=line}
}}}
What if `scriptLoop` alters other components of the state? Even if that
can't happen now, I still think it would be better to to get the state
again at the end.
I don't think the right line numbers are printed for errors inside
multiline commands. In fact, if everything was working nicely then I think
this would set the initial line_number to 0:
{{{
+ lift $ setGHCiState st{progname=filename,line_number=0}
}}}
I tried to fix this all up, but it was a deeper hole than I expected; I
wonder if the code might benefit from a little refactoring. It might also
be better to split the patch into two: the first bit just adding the
`:script` command, and the second improving the error message locations.
Are you interested in working more on this?
--
Ticket URL: <http://hackage.haskell.org/trac/ghc/ticket/1363#comment:17>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler
_______________________________________________
Glasgow-haskell-bugs mailing list
Glasgow-haskell-bugs@haskell.org
http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
