Add support for pandoc-2 by cdepillabout · Pull Request #40 · pbrisbin/yesod-markdown

cdepillabout · 2017-12-21T14:20:05Z

This PR adds a couple things:

Support for pandoc-2.
- pandoc-2's API is somewhat different than pandoc-1. The big differences affecting yesod-markdown are in the ReaderOptions and WriterOptions. I wasn't sure exactly how ReaderOptions and WriterOptions from pandoc-1 map to ReaderOptions and WriterOptions from pandoc-2. I played around with a couple different setups, and the current state of the code is the closest I got mapping from pandoc-1 to pandoc-2. If someone else is more well-versed in the Pandoc source code, there may be additional options to add/enable that will make it output exactly like it did originally.
- Another small change is the handleError function. handleError in pandoc-1 has the following type: Either PandocError a -> a, while in pandoc-2 it looks like this: Either PandocError a -> IO a. I've changed the yesod-markdown code to use unsafePerformIO with handleError. This should give us the same functionality as with pandoc-1.
Bumped the lts version to 10.0. This uses pandoc-2. I've also updated the version of stack used in the circle.yml file. I believe this is necessary for using lts-10.0.
Add a Changelog.md file.
Add the Changelog.md file and the README.md file to the .cabal file.
Bumped the version of yesod-markdown to 0.11.5. This should make it easy to cut a new release.

pbrisbin

Thanks for doing this! Please ping me when the review comments have been addressed and I'll happily (discuss further or) merge/release.

pbrisbin · 2017-12-21T14:34:35Z

Changelog.md

@@ -0,0 +1,4 @@
+
+## 0.11.5


Don't bump the version or write the changelog entry please. My practice is to do that after merge, in case multiple changes/PRs are going into one new version..

Okay, sounds good. I'll take that out.

Okay, I've removed the commit where I add a changelog and bump the version number.

pbrisbin · 2017-12-21T14:35:14Z

src/Yesod/Markdown.hs

  -- * Option sets
  , yesodDefaultWriterOptions
  , yesodDefaultReaderOptions
+  , yesodDefaultOptionExtensions


Can you call this yesodDefaultExtensions to be consistent with the other functions that are all yesodDefault{PandocType}

Sounds good. I'll make a change.

I've changed the name of yesodDefaultOptionExtensions to just yesodDefaultExtensions.

pbrisbin · 2017-12-21T14:36:06Z

src/Yesod/Markdown.hs

 import Data.Monoid (Monoid)
 #endif

+#if MIN_VERSION_pandoc(2,0,0)


I don't know that this package is important/popular enough to warrant such attention to backwards compatibility. I'd rather the code be simpler, cut a new major version, and just say yesod-markdown >= x for pandoc-2, yesod-markdown < x for pandoc-1.

That definitely sounds simpler. Let's do that.

I've changed the version bounds to to be pandoc >= 2.0 && < 2.1.

pbrisbin · 2017-12-21T14:41:09Z

src/Yesod/Markdown.hs

+handleErr :: Either PandocError a -> a
+handleErr =
+#if MIN_VERSION_pandoc(2,0,0)
+  unsafePerformIO . handleError


I think instead of using unsafePerformIO we should just write and use the pandoc-1 handleError ourselves (which uses error):

-- | An unsafe method to handle `PandocError`s. handleError :: Either PandocError a -> a handleError (Right r) = r handleError (Left err) = case err of ParseFailure string -> error string ParsecError input err' -> let errPos = errorPos err' errLine = sourceLine errPos errColumn = sourceColumn errPos ls = lines input ++ [""] errorInFile = if length ls > errLine - 1 then concat ["\n", (ls !! (errLine - 1)) ,"\n", replicate (errColumn - 1) ' ' ,"^"] else "" in error $ "\nError at " ++ show err' ++ errorInFile

I'm not happy about it, but

It's the only way we can support ToMarkup Markdown, which is really a primary use-case

At least this preserves the exact same form of (un)safety from the versions before

WDYT?

I can try to get something like this working. PandocError from pandoc-2 has a bunch of extra error cases, so we'll have to handle those as well in our handleError function:

data PandocError = PandocIOError String IOError | PandocHttpError String HttpException | PandocShouldNeverHappenError String | PandocSomeError String | PandocParseError String | PandocParsecError Input ParseError | PandocMakePDFError String | PandocOptionError String | PandocSyntaxMapError String | PandocFailOnWarningError | PandocPDFProgramNotFoundError String | PandocPDFError String | PandocFilterError String String | PandocCouldNotFindDataFileError String | PandocResourceNotFound String | PandocTemplateError String | PandocAppError String | PandocEpubSubdirectoryError String | PandocMacroLoop String deriving (Show, Typeable, Generic) -- | Handle PandocError by exiting with an error message. handleError :: Either PandocError a -> IO a handleError (Right r) = return r handleError (Left e) = case e of PandocIOError _ err' -> ioError err' PandocHttpError u err' -> err 61 $ "Could not fetch " ++ u ++ "\n" ++ show err' PandocShouldNeverHappenError s -> err 62 s PandocSomeError s -> err 63 s PandocParseError s -> err 64 s PandocParsecError input err' -> let errPos = errorPos err' errLine = sourceLine errPos errColumn = sourceColumn errPos ls = lines input ++ [""] errorInFile = if length ls > errLine - 1 then concat ["\n", ls !! (errLine - 1) ,"\n", replicate (errColumn - 1) ' ' ,"^"] else "" in err 65 $ "\nError at " ++ show err' ++ -- if error comes from a chunk or included file, -- then we won't get the right text this way: if sourceName errPos == "source" then errorInFile else "" PandocMakePDFError s -> err 65 s PandocOptionError s -> err 2 s PandocSyntaxMapError s -> err 67 s PandocFailOnWarningError -> err 3 "Failing because there were warnings." PandocPDFProgramNotFoundError pdfprog -> err 47 $ pdfprog ++ " not found. Please select a different --pdf-engine or install " ++ pdfprog PandocPDFError logmsg -> err 43 $ "Error producing PDF.\n" ++ logmsg PandocFilterError filtername msg -> err 83 $ "Error running filter " ++ filtername ++ ":\n" ++ msg PandocCouldNotFindDataFileError fn -> err 97 $ "Could not find data file " ++ fn PandocResourceNotFound fn -> err 99 $ "File " ++ fn ++ " not found in resource path" PandocTemplateError s -> err 5 s PandocAppError s -> err 1 s PandocEpubSubdirectoryError s -> err 31 $ "EPUB subdirectory name '" ++ s ++ "' contains illegal characters" PandocMacroLoop s -> err 91 $ "Loop encountered in expanding macro " ++ s err :: Int -> String -> IO a err exitCode msg = do UTF8.hPutStrLn stderr msg exitWith $ ExitFailure exitCode return undefined

Gotcha. I think I'm OK with

handleError :: Either PandocError a -> a handleError = either (error . show) id

Sorry if you put a lot of time into the more robust version...

Okay, no problem. Luckily I haven't started working on it yet :-)

Let's go with this simpler version.

I've changed the handleError function to be as you've described above.

cdepillabout · 2017-12-22T04:44:29Z

Hmm, the circleci builds seem to be failing because of using too much memory.

Would you consider switching over to Travis? There is a page on using stack with Travis, and it includes a sample .travis.yml.

pbrisbin

Awesome. This LGTM.

Don't worry about CI. I generally very much prefer Circle over Travis, so I'd like to make it work -- but these OOM errors have been cropping up lately on my Haskell projects. I'm going to cut a new branch that (among other things) moves to Circle 2.0, and we'll see how it does there.

Either way, I'll take this from here -- thanks!

cdepillabout · 2017-12-22T13:53:31Z

Thanks, sounds good! 👍

pbrisbin · 2017-12-22T15:43:44Z

@cdepillabout I'm getting errors like the following:

/home/patrick/code/pbrisbin/yesod-markdown/rts/posix/OSThreads.c:137:0: error:
     error: undefined reference to 'pthread_create'

both locally and on CI.

Do you know anything about this, or how to fix?

pbrisbin · 2017-12-22T18:50:47Z

Closing this to consolidate work in #42. Feel free to keep discussing that error here or there, I don't mind either way.

pbrisbin reviewed Dec 21, 2017

View reviewed changes

cdepillabout added 3 commits December 22, 2017 12:06

Add support for pandoc-2.

f2b4fda

Bump lts version to lts-10.0.

52507a4

Add README.md to extra-source-files in cabal file.

99b25f7

cdepillabout force-pushed the pandoc-2 branch from 659944f to 99b25f7 Compare December 22, 2017 03:08

pbrisbin approved these changes Dec 22, 2017

View reviewed changes

pbrisbin mentioned this pull request Dec 22, 2017

Circle 2.0, pandoc-2.0, lts-10.0 #42

Merged

pbrisbin closed this Dec 22, 2017

cdepillabout deleted the pandoc-2 branch December 23, 2017 00:32

Conversation

cdepillabout commented Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pbrisbin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbrisbin Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdepillabout commented Dec 22, 2017

Uh oh!

pbrisbin left a comment

Choose a reason for hiding this comment

Uh oh!

cdepillabout commented Dec 22, 2017

Uh oh!

pbrisbin commented Dec 22, 2017

Uh oh!

pbrisbin commented Dec 22, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cdepillabout commented Dec 21, 2017 •

edited

Loading

pbrisbin Dec 21, 2017 •

edited

Loading