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

Readers.RST should keep the figure name option to allow using pandoc-crossref and pandoc-fignos with rst input #5619

Closed
ociule opened this issue Jun 24, 2019 · 12 comments · Fixed by #5637

Comments

@ociule
Copy link

ociule commented Jun 24, 2019

pandoc-crossref and pandoc-fignos are great tools that enable numbering figures and tables and referencing them. Unfortunately, they only work for markdown input.

However, I've become convinced that it would be quite easy to allow pandoc RST input users to allow numbering figures and tables. All is needed for this is for the RST reader to keep the id option for these directives.

Let's take a small md fragment from the fignos demo:

Reference to @fig:1.

![The number one.](img1.jpg){#fig:1 width=1in}

We can look at the equivalent native code with pandoc fignos_demo.md -o fignos_demo_md.native:

[Para [Str "Reference",Space,Str "to",Space,Cite [Citation {citationId = "fig:1", citationPrefix = [], citationSuffix = [], citationMode = AuthorInText, citationNoteNum = 0, citationHash = 0}] [Str "@fig:1"],Str "."]
,Para [Image ("fig:1",[],[("width","1in")]) [Str "The",Space,Str "number",Space,Str "one."] ("img1.jpg","fig:")]
]

And this works like expected (the figure is numbered and the reference becomes a link) with fignum and crossref, like we can test with pandoc -F pandoc-fignos fignos_demo_md.native -o fignos_demo.html

The roughly equivalent rst would be (without the citation, because it seems Readers.RST does not have support for that):

.. figure:: img1.jpg
  :name: The number one.
  :fig: 1
  :width: 1in

  The number one.

Which with pandoc 2.7 becomes the following native code:

[Para [Image ("",[],[("width","1in")]) [Str "The",Space,Str "number",Space,Str "one."] ("img1.jpg","fig:")]]

Note that there is no Image id (no more "fig:1" after Image), which stops fignos or crossref from working with rst input. Simply adding it to the native format enables figure numbering!

This is obviously related to the great discussion about internal links in #813

It would be great if the RST reader keeps the "fig:1" after Image (not 100% sure if id is the right name for this attribute), so that the native output from parsing this rst would include it. Then, fignos would be capable of numbering rst figures too. References to those figures still would not work, I think, without more work on rst citations, but one fix at a time.

I would love to propose a patch, but I don't know enough Haskell.

The obvious place to patch is https://github.com/jgm/pandoc/blob/master/src/Text/Pandoc/Readers/RST.hs#L727

There are several places in Readers.Markdown that use imageWith, but I could not be sure which was the right one to use for inspiration. Don't hesitate to point it out please.

The equivalent work could be done for tables too.

@ociule ociule changed the title Readers.RST should keep the figure, image and table id option to allow using pandoc-crossref, pandoc-fignum and pandoc-tablenum with rst input Readers.RST should keep the figure and table id option to allow using pandoc-crossref, pandoc-fignum and pandoc-tablenum with rst input Jun 24, 2019
@mb21
Copy link
Collaborator

mb21 commented Jun 25, 2019

Isn't this issue basically a subset of #813 ?

Besides the architectural questions, we currently don't even have attributes on tables (#1024)... but yes, we're thinking/working on that. You're welcome to join the discussion on those issues...

@ociule
Copy link
Author

ociule commented Jun 25, 2019

I am aware of the discussion in #813. Solving #813 needs a lot of dev effort, and this is a part of that. #813 focuses a lot on Markdown (there's only one comment mentioning rst at all, and no comments mentioning other formats!)

This issue offers a lof of value (the ability to work with existing tooling like crossref, fignos and tablenos for rst input) for what looks like not a lot of dev effort - not trying to presume too much, but it seems it's a local patch in Readers.RST.

@mb21
Copy link
Collaborator

mb21 commented Jun 25, 2019

it seems it's a local patch in Readers.RST

For the figure you might be correct (although this might also be better done after #3177). For the table, as I mentioned before, there is no place to store the id in pandoc's current document AST.

@ociule
Copy link
Author

ociule commented Jun 25, 2019

there is no place to store the id in pandoc's current document AST

I see. There is some hope yet: pandoc-tablenos nevertheless works for md input! Which obviously uses the pandoc AST Table element. So the solution for rst input should copy the mechanism the md solution uses.

If I read https://github.com/tomduck/pandoc-tablenos/blob/master/pandoc_tablenos.py#L177 correctly, it seems tablenos finds the table identifier in attrs[0]. This is apparently the first value in the 5 arguments a Table constructor gets, as we can see in https://github.com/jgm/pandocfilters/blob/master/pandocfilters.py#L264

I have no idea how this squares with the discussion in #1024. tablenos clearly has access to a table id attribute defined like this in markdown:

  Right     Left     Center     Default
-------     ------ ----------   -------
     12     12        12            12
    123     123       123          123
      1     1          1             1

Table: Demonstration of a simple table. {#tbl:a}

Without the {#tbl:a}, tablenos does not number that table.

So Readers.RST could put the table id in attrs[0], like Readers.Markdown does.

Now we need to choose, similar to what took place in #813 for markdown, where to put the id value in rst table and figure directives. A comment in #813 proposed using the already existing name option. A name can have spaces, I'm not sure that's ok in an id value. A fig option for figures and tbl for tables would be out of the box compatible with fignos and crossref. It would need less special case handling in Readers.RST: we just take all directive options and put them in attributes.

@ociule
Copy link
Author

ociule commented Jun 25, 2019

Found the mechanism that gives tablenos access to a table id attribute. It cheats. Crossref must do the same.

The markdown table example above becomes the following native AST:

[Table [Str "Demonstration",Space,Str "of",Space,Str "a",Space,Str "simple",Space,Str "table.",Space,Str "{#tbl:a}"] [AlignRight,AlignLeft,AlignCenter,AlignDefault] [0.0,0.0,0.0,0.0]
 [[Plain [Str "Right"]]
...

As you can see, the table id is not parsed by pandoc, it's left as Str in the Table caption. Somehow pandocfilters (the python library for building pandoc filters like crossref and tablenos) is parsing it from the caption Str on its own 😆

@ociule ociule changed the title Readers.RST should keep the figure and table id option to allow using pandoc-crossref, pandoc-fignum and pandoc-tablenum with rst input Readers.RST should keep the figure id option to allow using pandoc-crossref and pandoc-fignum with rst input Jun 25, 2019
@ociule
Copy link
Author

ociule commented Jun 25, 2019

FWIW, pandoc-tablenos already works with RST input, as the trick it uses for md (appending a {#tbl:id} to the table caption) works with rst too.

See tomduck/pandoc-fignos#69 (comment)

I'm editing the title to restrict the scope to figures only.

@jgm
Copy link
Owner

jgm commented Jun 25, 2019

I don't see any documentation of a fig: field for RST figures at
http://docutils.sourceforge.net/docs/ref/rst/directives.html#figure

Is this ad hoc nonstandard RST, then? I think we should stick to documented behavior of RST.

@ociule
Copy link
Author

ociule commented Jun 26, 2019

@jgm Yes, unfortunately, that was ad hoc RST. The least ad-hoc rst approach seems to be using the name field.

Remember that this new field must be optional, per figure, so that some can be numbered and some not. That restricts what existing fields can be reused or piggybacked upon.

We could piggyback on the figure caption field. We only want to number captioned figures anyway (this is what fignos does - no caption, no number).

But let's look at using the name field, as it looks to be ignored by pandoc. It is not output to the native format, for example.

.. figure:: img1.jpg
  :width: 1in
  :name: test

  The caption. Here's what piggybacking on caption would look like {#fig:1}

Becomes the following native format, without name:

[Para [Image ("",[],[("width","1in")]) [Str "The",Space,Str "caption.",Space,Str "Here's",Space,Str "what",Space,Str "piggybacking",Space,Str "on",Space,Str "caption",Space,Str "would",Space,Str "look",Space,Str "like",Space,Str "{#fig:1}"] ("img1.jpg","fig:")]]

So using the name field for this seems like the least ad-hoc rst approach. All rst directives have a name, even if it's not explicitly mentioned in the RST figure doc. It is mentioned above for the image directive.

A further argument for name is that it's supposed to be used as a hyperlink target value anyway, so it has the right semantics already:

Specifying the name option of a directive, e.g.,

.. image:: bild.png
   :name: my picture
is a concise syntax alternative to preceding it with a hyperlink target

.. _my picture:

.. image:: bild.png
New in Docutils 0.8.

@jgm
Copy link
Owner

jgm commented Jun 27, 2019

I agree, using name seems the right approach here.
We already parse this as the identifier with most directives.

@leungbk
Copy link
Contributor

leungbk commented Jun 28, 2019

imgAttr cl = ("", classes ++ alignClasses, widthAttr ++ heightAttr)

Changing the blank string to name here yields the following:

$ pandoc -f rst -t native
.. figure:: img1.jpg
  :width: 1in
  :name: test

  The caption. Here's what piggybacking on caption would look like {#fig:1}
^D
[Para [Image ("test",[],[("width","1in")]) [Str "The",Space,Str "caption.",Space,Str "Here's",Space,Str "what",Space,Str "piggybacking",Space,Str "on",Space,Str "caption",Space,Str "would",Space,Str "look",Space,Str "like",Space,Str "{#fig:1}"] ("img1.jpg","fig:")]]

@ociule is this what you're looking for?

@ociule
Copy link
Author

ociule commented Jul 1, 2019

@leungbk that's exactly it! This change would give pandoc-fignos what it needs to work correctly.

Thanks for the dev work. Hope it can be approved for merging.

@ociule ociule changed the title Readers.RST should keep the figure id option to allow using pandoc-crossref and pandoc-fignum with rst input Readers.RST should keep the figure id option to allow using pandoc-crossref and pandoc-fignos with rst input Jul 1, 2019
@ociule ociule changed the title Readers.RST should keep the figure id option to allow using pandoc-crossref and pandoc-fignos with rst input Readers.RST should keep the figure name option to allow using pandoc-crossref and pandoc-fignos with rst input Jul 1, 2019
leungbk added a commit to leungbk/pandoc that referenced this issue Jul 8, 2019
@jgm jgm closed this as completed in #5637 Jul 11, 2019
jgm pushed a commit that referenced this issue Jul 11, 2019
@ociule
Copy link
Author

ociule commented Jul 11, 2019

Thanks for solving this guys! pandoc is a great tool and you keep making it better!

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

Successfully merging a pull request may close this issue.

4 participants