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 support for number_offset metadata with number-sections.lua #1999

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

atusy
Copy link
Collaborator

@atusy atusy commented Dec 25, 2020

number-sections.lua enables numbering sections for formats without it.
However, current implementation lacks the support for --number-offset.
Unfortunately, there is no way to tell Lua filter the value of --number-offset.
Thus, this PR reads number-offset metadata as an alternative.

Reprex:

---
output:
  md_document:
    number_sections: true
number_offset: 3
---

# foo

@atusy
Copy link
Collaborator Author

atusy commented Dec 25, 2020

Maybe we should consider consistent behaviors on formats with native --number-sections option.

---
output:
  html_document:
    number_sections: true
number_offset: 3
---

# foo

@atusy
Copy link
Collaborator Author

atusy commented Dec 25, 2020

Maybe we should consider consistent behaviors on formats with native --number-sections option.

done

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this PR, and will let @cderv decide whether to merge it. Thanks!

Unfortunately, there is no way to tell Lua filter the value of --number-offset.

Yes, that's unfortunate. It will be great if we don't have to introduce a new field in YAML.

@yihui yihui requested a review from cderv May 18, 2021 03:12
@cderv cderv self-assigned this May 18, 2021
@cderv
Copy link
Collaborator

cderv commented Jun 2, 2021

I took time on this one because I needed to think about it.

I really don't like adding a new YAML field, all the more not under a format function but at the first level in the YAML. This usually is to pass variable to pandoc. It is already not quite easy to follow which field will be processed by which tool so I am not sure about this.

But even considering that, I thought deeper about the why we would need this.
Is --number-offset often used with rmarkdown project ? I never had the need for this command line flag, and not sure to see in which case it would be useful. Pandoc itself only supports it for HTML document
From https://pandoc.org/MANUAL.html#option--number-offset

Offset for section headings in HTML output (ignored in other output formats).

This means that --number-offset is ignored for LaTeX conversion and number-sections.lua is not used in pdf_document(). So even if we support this, one format would still lack the feature, unless we use number-sections.lua for pdf_document() also, but we are redoing Pandoc feature in Lua in this case, aren't we ?

Maybe this option to offset the numbering is only useful when producing HTML documents ?

Is there a use case behind this PR @atusy or just a quest of completeness ?
As @yihui taught me, sometimes it is better to wait for a feature to be asked even if we have the idea to do it in the first place.

If we were to do it, I would prefer having the field be at the same level of number_sections

---
output:
  md_document:
    number_sections: true
    number_offset: 3
---

I may be wrong but I assume that field under output are mainly processed by R, whereas other field are mainly for Pandoc to use directly.

I would also add this in html_document(), but it would still be missing for PDF outputs.
I leverage the fact that metadata can be passed to Lua filter at command line and not necessarily to be set in YAML header in the first place.

What are your thoughts on all this ?

@atusy
Copy link
Collaborator Author

atusy commented Jun 3, 2021

Thanks @cderv for your insightful comments.

I understand completeness is not always the priority and am fine to close this PR.
Yet, let me tell you my minor use case.

I am currently co-authoring a book with R Markdown, and each authors render .md files per their chapters.
There, we need section numbers and cross reference with number_offset, and I implemented the features with Lua filter.

@cderv
Copy link
Collaborator

cderv commented Jun 3, 2021

Oh that is interesting. So you are running pandoc on each file ? and not on all separate file at once ?
I see that this can be useful in this case. I am surprised that Pandoc itself does not cover this.

How does it work with PDF ? Do you use the Lua filter instead of the --number-sections support from Pandoc ?

@atusy
Copy link
Collaborator Author

atusy commented Jun 4, 2021

Yes, we run pandoc on each file.

Actually, conversion to PDF is not author's scope.
We just submit markdown files to our editor.
I do not know his ecosystem in detail.
I know that he at least uses Adobe InDesign.

@cderv
Copy link
Collaborator

cderv commented Jun 4, 2021

ok. Thanks for sharing!

@cderv
Copy link
Collaborator

cderv commented Feb 9, 2022

FYI Pandoc 2.17 has now added a Global Variable PANDOC_WRITER_OPTIONS that contains the writer options, among them number_offset.

Table of the options that will be passed to the writer. While the object can be modified, the changes will not be picked up by pandoc

Maybe this would prevent us for adding a new first level YAML field for the Lua Filter to work. Which is mainly why I am holding merging this PR for now, waiting for a better solution. That may be it.

@atusy
Copy link
Collaborator Author

atusy commented Feb 11, 2022

@cderv

Thank you for good news! I think it is okay to support number_offset only with Pandoc >= 2.17.
I'll update the implementation.

@cderv
Copy link
Collaborator

cderv commented Feb 11, 2022

Cool ! Let me know if this enough - I never used this variable before. Hopefully it will work well for this ! And open room for other feature in Lua!

@cderv cderv assigned atusy and unassigned cderv Feb 11, 2022
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.

3 participants