Use the SimpleFigure in Readers
#7611
Conversation
| , ("figure", env "figure" $ skipopts *> Text.Pandoc.Readers.LaTeX.figure) | ||
| , ("subfigure", env "subfigure" $ skipopts *> tok *> Text.Pandoc.Readers.LaTeX.figure) |
There was a problem hiding this comment.
These qualifications won't be necessary until figure is added to Builder, correct?
| -- For the MediaWiki format all images are considered figures | ||
| [Image attr figureCaption (src, title)] -> | ||
| return $ B.simpleFigureWith | ||
| attr (B.fromList figureCaption) src title |
There was a problem hiding this comment.
Can you elaborate on "For the MediaWiki format all images are considered figures"? Is this a change in behavior? What motivates it?
There was a problem hiding this comment.
The original (before I rewrote it using simple figure) code had that behavior and I kept it. I just implemented the behavior here (at lines ~205-208)
| return $ B.imageWith attr fname ("fig:" <> stringify caption) caption | ||
| return $ B.imageWith attr fname (stringify caption) caption |
There was a problem hiding this comment.
Related to the previous one this is where the "For the MediaWiki format all images are considered figures" came from. The original code used the "fig:" prefix all the time as the deleted 634 line shows.
MediaWiki supports inline images: https://www.mediawiki.org/wiki/Help:Images
I just kept the old behavior. (but implemented it at lines ~206)
There was a problem hiding this comment.
This patch's version
$ pandoc -f mediawiki -t native
this [[File:example.jpg|caption]] is
[ Para
[ Str "this"
, Space
, Image
( "" , [] , [] )
[ Str "caption" ]
( "example.jpg" , "caption" )
, Space
, Str "is"
]
]
pandoc 2.14.1
$ pandoc -f mediawiki -t native foo.mediawiki
[Para [Str "this",Space,Image ("",[],[]) [Str "caption"] ("example.jpg","fig:caption"),Space,Str "is"]]
Notice the 'fig:' prefixing the caption.
It doesn't actually create a figure ([Para [Image ..]]) but it adds a superfluous (?) 'fig:'. This patch omits the fig:
So the comment should be removed\
There was a problem hiding this comment.
ok, thanks for clarifying, this makes sense.
| let fig = "figure:: " <> literal src | ||
| alt = ":alt: " <> if T.null tit then description else literal tit | ||
| capt = description | ||
| (_,cls,_) = attr | ||
| classes = case cls of | ||
| [] -> empty | ||
| ["align-right"] -> ":align: right" | ||
| ["align-left"] -> ":align: left" | ||
| ["align-center"] -> ":align: center" | ||
| _ -> ":figclass: " <> literal (T.unwords cls) | ||
| return $ hang 3 ".. " (fig $$ alt $$ classes $$ dims $+$ capt) $$ blankline | ||
| blockToRST (Para [Image attr txt (src, _)]) = do | ||
| description <- inlineListToRST txt | ||
| dims <- imageDimsToRST attr | ||
| let fig = "image:: " <> literal src | ||
| alt | null txt = empty | ||
| | otherwise = ":alt: " <> description | ||
| capt | isfig = description | ||
| | otherwise = empty | ||
| capt = empty | ||
| (_,cls,_) = attr | ||
| classes = case cls of | ||
| [] -> empty | ||
| ["align-right"] -> ":align: right" | ||
| ["align-left"] -> ":align: left" | ||
| ["align-center"] -> ":align: center" | ||
| _ | isfig -> ":figclass: " <> literal (T.unwords cls) | ||
| | otherwise -> ":class: " <> literal (T.unwords cls) | ||
| _ -> ":class: " <> literal (T.unwords cls) | ||
| return $ hang 3 ".. " (fig $$ alt $$ classes $$ dims $+$ capt) $$ blankline | ||
| blockToRST (Para inlines) | ||
| | LineBreak `elem` inlines = |
There was a problem hiding this comment.
Can we consolidate some of the duplicated code used here to handle image attributes?
Also, what if there are multiple classes, one of which is e.g. align-center? we probably shouldn't assume there will just be one.
There was a problem hiding this comment.
Although for this iteration of the code we (tarleb and me) intended to keep the original behavior as much as possible. I'll see to consolidate the code in this commit and then make another commit that considers the possibility of multiple classes.
There was a problem hiding this comment.
Okay, fair enough, if the old code had the same issue, then this change doesn't need to fix it.
| @@ -406,7 +412,7 @@ blockListToRST' topLevel blocks = do | |||
| toClose Header{} = False | |||
| toClose LineBlock{} = False | |||
| toClose HorizontalRule = False | |||
| toClose (Para [Image _ _ (_,t)]) = "fig:" `T.isPrefixOf` t | |||
| toClose SimpleFigure{} = True | |||
|
Great, thanks for updating this! I've made a few small comments, but this seems close to ready. |
|
I'd like to get this merged before we need to do more rebasing. |
|
Sorry for the delay jgm, I'll work on this in the following hours. |
|
I went ahead and merged this manually. |
Hi,
This is the first part of #7364. It uses the
SimpleFigurepattern synonym in the readers without changing the output.The
SimpleFigurepattern synonym was merged into pandoc-types here.