diagrams / diagrams-haddock

Preprocessor for including inline diagrams in Haddock documentation
Other
10 stars 4 forks source link

Processing kills file when an HSE exception happens. #3

Closed fryguybob closed 11 years ago

fryguybob commented 11 years ago

When parsing causes an exception due to a bug in HSE we die while writing the file.

ghci> :! cat test.hs
{-# LANGUAGE TypeFamilies #-}
-- <<blah#diagram=ha>>
-- > ha = circle 1
instance a ~ b => Blah a
ghci> import Diagrams.Haddock
ghci> processHaddockDiagrams "./" "./" "test.hs"
*** Exception: src/Language/Haskell/Exts/Annotated/ExactPrint.hs:(952,9)-(956,22): Non-exhaustive patterns in case

ghci> :! cat test.hs
ghci>
byorgey commented 11 years ago

Aha! I think I see what is going on. I said earlier that I thought the checks to see whether the file has changed would force the file contents to be generated before opening the output file for writing. However, that's only true if nothing has actually changed! In the case that something has changed (as the <<blah#diagram=ha>> part will above), the test can stop early and the remainder of the file is not forced until it is actually being written.

One solution would be to use deepseq to force the entire output before writing. However, as discussed in #diagrams, I think a better solution (which also fixes some other problems -- e.g. makes it possible to deal with CPP and doesn't alter trailing whitespace) is to stop relying on HSE for pretty-printing the file. Instead, once we have parsed the input file with HSE, picked out diagram URLs and code blocks, and compiled all the needed diagrams, we can re-parse the entire input file using parseDiagramURLs, modify the URLs, and then serialize it back out. (Technically this could do something slightly wrong if the file happens to contain a diagram URL inside a string literal, but that seems quite unlikely.) This way we can be relatively certain that generating the output cannot throw an exception. We have to make sure that HSE's parsing is fully forced before opening the output file, but I think that is not an issue, since parsing returns a Maybe which we pattern-match on; presumably matching on the Maybe forces the entire input file to be parsed, since it cannot know whether parsing will be successful otherwise.

Another thing we could do (which is probably the Right Thing To Do (tm), and might be a good idea independently) is to write to a temporary file and only copy the temporary file over the original file once we are sure it has been written properly. In fact I seem to recall a Hackage package that facilitates this sort of thing, but I can't find it at the moment.

byorgey commented 11 years ago

Aha, I found the package I was thinking of: http://hackage.haskell.org/package/cautious-file . Looks perfect.