Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the SimpleFigure bidirectional pattern synonym. #93

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

argent0
Copy link
Contributor

@argent0 argent0 commented Sep 15, 2021

Technically not API-Change ?

The first change part of #90

To address some of the issues with the previous handling of figures, we introduced, along with some helper functions, the SimpleFigure pattern synonym

-- | Constructor for a figure with a single image.
--
-- It can be used to construct a figure:
--
-- >>> SimpleFigure nullAttr [] (T.pack "", T.pack "title")
-- Para [Image ("",[],[]) [] ("","fig:title")]
--
--
-- It can be used to pattern match:
--
-- >>> let img = Para [Image undefined undefined (undefined, T.pack "title")]
-- >>> case img of { SimpleFigure _ _ _ -> True; _ -> False }
-- False
-- >>> let fig = Para [Image undefined undefined (undefined, T.pack "fig:title")]
-- >>> case fig of { SimpleFigure _ _ tit -> snd tit; _ -> T.pack "" }
-- "title"
pattern SimpleFigure :: Attr -> [Inline] -> Target -> Block
pattern SimpleFigure attributes figureCaption tgt <-
    Para [Image attributes figureCaption
        (isFigureTarget -> Just tgt)]  where
  SimpleFigure attributes figureCaption tgt =
    Para [Image attributes figureCaption (second ("fig:" <>) tgt)]

This is very much like adding a new constructor to the Block type but with some differences.

Some benefits of this approach are:

  • It is backward compatible with the previous construction.

  • It formalizes, without enforcing, the roles for the construction's elements.

  • It makes it easier to find where are figures handled in the code for both Readers and Writers.

    Here is an example diff from the RST Reader in the pandoc pull request:

    - return $ B.para (B.imageWith (imgAttr "figclass") src "fig:"
    -             caption) <> legend
    + return $ B.simpleFigureWith
    +     (imgAttr "figclass") caption src "" <> legend

Of course, there are also some limitations:

  • It doesn't enforce the construction on new code, as an actual constructor would do. For example, code with non-exhaustive patterns matches won't rise a warning if SimpleFigure is not handled.

  • It lacks an explicit alt-text field. This can be included using attributes, but I would have liked to give it more importance.

I believe this is a modest improvement of the previous code. It provides an explicit representation for figures. It could go a little further with an explicit constructor; but keeping the behavior backward compatible prevents breaking some workflows down the line, for example someone using a lua-filter.

@jgm jgm merged commit e2e62e3 into jgm:master Sep 17, 2021
@jgm
Copy link
Owner

jgm commented Sep 17, 2021

thanks!

@jgm
Copy link
Owner

jgm commented Sep 17, 2021

PS This is actually an API change (anything that changes in the exports is), though not an AST change.
It would have been good to mark it as such in the commit message, just to make sure I record that in the changelog - but I'm sure I'll remember.

@argent0
Copy link
Contributor Author

argent0 commented Sep 20, 2021

Thanks!

@jgm
Copy link
Owner

jgm commented Sep 20, 2021

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants