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

Endnotes #4042

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Endnotes #4042

wants to merge 1 commit into from

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Nov 5, 2017

This is for issue #4041 and depends on jgm/pandoc-types#34

So far I did some straightforward changes, trying to relpace Note with Note FootNote where possible. For some reason Lua tests fail.

Reproduce with: stack test -j 1 --test-arguments '-p Lua' 2>&1 | less

Can someone familiar with lua (@tarleb ?) help with fixing this? Looks like something should be done about function Note, for example replacing it with function FootNote and function EndNote like it is done with Math.

@link2xt link2xt changed the title Endnotes [WIP] Endnotes Nov 5, 2017
@tarleb
Copy link
Collaborator

tarleb commented Nov 5, 2017

Don't have time to look at it right now, but the most likely explanation is that you'll have to edit the M.Note function in data/pandoc.lua.

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 5, 2017

@tarleb Thanks, trying to fix it.

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 5, 2017

Fixed, all tests pass. Now going to add endnotes to Docx and ODT.

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 6, 2017

Reporting progress:
Still need to fix Docx writer.
Also maybe add some tests for OpenDocument writer or add EndNote to generic writer test at least. Maybe the best option is to check for round-trip, like Docx does now.

@link2xt link2xt force-pushed the endnotes branch 3 times, most recently from 3d91574 to effc480 Compare November 6, 2017 20:19
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 6, 2017

All done, need review.

Especially for Docx writer. Documents it produces can be loaded by LibreOffice, that is all I have tested. Code is mostly a copy of footnotes, I think it can be hlinted and pruned after merge. I don't understand the reason for defaultFootnotes, so I left default endnotes list empty. Judging from comments, looks like a kludge for some MS Office glitch? OOXML standard part 1 is 5000+ pages, TLDR.

Also, I will have to switch from my repo back to official one, for that we need a new version of pandoc-types, right? So first jgm/pandoc-types#34 is to be merged and a new version assigned, then I can fix first commit in this PR.

@link2xt
Copy link
Collaborator Author

link2xt commented Dec 15, 2017

Added support for Muse ({1} for endnotes). Now ODT, Docx and Muse support endnotes both for reading and writing. CI failures are due to the fact that I use custom version of pandoc-types.

@jgm How do I proceed to get this PR merged? I think it is ready.

@@ -232,7 +232,9 @@ inlineToCustom (Link attr txt (src,tit)) =
inlineToCustom (Image attr alt (src,tit)) =
callFunc "Image" (Stringify alt) src tit (attrToMap attr)

inlineToCustom (Note contents) = callFunc "Note" (Stringify contents)
inlineToCustom (Note FootNote contents) = callFunc "FootNote" (Stringify contents)
Copy link
Owner

Choose a reason for hiding this comment

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

This approach will break existing custom writers that use Note.
A better approach would be callFunc "Note" "footnote" and callFunc "Note" "endnote", I think.
This would allow current writers to work without being changed. @tarleb, any thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems like a nice solution. We'd get full backwards compatibility if we pass the note type after the content:

inlineToCustom (Note noteType contents) = callFunc "Note" (Stringify contents) noteType

Lua won't mind the extra parameter if the function just expects contents. This would pass the node type as "FootNote" and "EndNote*.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@link2xt link2xt force-pushed the endnotes branch 2 times, most recently from 35930b9 to 8c38331 Compare December 30, 2017 22:43
@link2xt
Copy link
Collaborator Author

link2xt commented Dec 30, 2017

@jgm ping, just updated PR

Linking #4213 as related

@link2xt link2xt changed the title [WIP] Endnotes Endnotes Jan 11, 2018
@link2xt link2xt force-pushed the endnotes branch 5 times, most recently from 8380f26 to 5b50995 Compare January 16, 2018 08:53
@link2xt
Copy link
Collaborator Author

link2xt commented Feb 13, 2018

Rebased it once again on top of refactored Muse reader.

@jkr How do I update binary Docx files with the new test framework? All binary tests are failing mostly due to added endnote style. Manual testing shows produced files with endnotes are ok.

@link2xt link2xt force-pushed the endnotes branch 2 times, most recently from 1c9175b to 917f3df Compare April 29, 2018 14:46
@link2xt
Copy link
Collaborator Author

link2xt commented Apr 29, 2018

@tarleb
I fixed Lua support and added a test for a filter that converts footnotes to endnotes.

@jgm I think it is ready. Custom writers are not broken, tested manually with pandoc -t data/sample.lua.

@tarleb
Copy link
Collaborator

tarleb commented May 1, 2018

@labdsf Thanks, LGTM. I'm only wondering why it is FootNote and EndNote instead of Footnote and Endnote.

@link2xt
Copy link
Collaborator Author

link2xt commented May 1, 2018

@tarleb

I'm only wondering why it is FootNote and EndNote instead of Footnote and Endnote.

Not sure, most likely because I looked at SingleQuote/DoubleQuote and DisplayMath/InlineMath while implementing it. This way quote, math and note types have capitalized "quote", "math" and "note" in their names, it is more consistent.

On the other hand, "footnote", "endnote" and "sidenote" are single words in English, while "doublequote" and "inlinemath" are not.

I still prefer the CamelCase style, because it allows to add something like "PopupNote", "MarginNote" etc.

@jgm
Copy link
Owner

jgm commented May 3, 2018 via email

@link2xt
Copy link
Collaborator Author

link2xt commented May 3, 2018

I have replaced FootNote and EndNote with Footnote and Endnote.

Had to replace Run constructors Footnote and Endnote with RunFootnote and RunEndnote in Docx to avoid conflicts.

@jgm
Copy link
Owner

jgm commented Jul 21, 2018

This is a massive PR, so it's difficult to examine thoroughly.

Can you clarify which readers and writers actually support the distinction between endnotes and footnotes? (I assume that the other readers and writers just treat all notes as footnotes?)

@link2xt
Copy link
Collaborator Author

link2xt commented Jul 21, 2018

@jgm Endnotes are supported in both readers and writers for ODT, Docx and Muse (with Amusewiki syntax: "{1}", "{2}" instead of "[1]" and "[2]" for footnotes), besides Native. All the other readers and writers simply ignore the type and treat them as footnotes.

Later it would be nice to add support for LaTeX, at least for the writer, but that requires using additional packages, so I have left it out for now. HTML/EPUB can get support once #4213 is resolved.

@link2xt link2xt force-pushed the endnotes branch 2 times, most recently from 13214d4 to f0790f5 Compare August 22, 2018 08:12
@link2xt
Copy link
Collaborator Author

link2xt commented Sep 2, 2018

@jgm
Just rebased it again to merge with Muse changes. Any updates?

@link2xt link2xt force-pushed the endnotes branch 2 times, most recently from c7fc26c to 4fbfda6 Compare September 17, 2018 06:20
@link2xt link2xt force-pushed the endnotes branch 3 times, most recently from b4e25e6 to 882def3 Compare October 19, 2018 17:07
@johannesburg
Copy link

I'm wondering if this PR is still being worked on? It would be great to be able to specify whether notes are endnotes and footnotes, especially for .docx files

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

Successfully merging this pull request may close these issues.

5 participants