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 a space delimiter across the style #346

Merged
merged 2 commits into from
Jun 11, 2020
Merged

Conversation

adam3smith
Copy link
Contributor

Afaik, display elements don't work properly with pandoc, so I don't think the block elements will do anything for you. citeproc-js isn't happy with those two blocks in there.

Afaik, display elements don't work properly with pandoc, so I don't think the block elements will do anything for you. citeproc-js isn't happy with those two blocks in there.
@AppVeyorBot
Copy link

AppVeyor build 1.0.140 for commit 7335415 is now complete.

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@rmzelle
Copy link
Contributor

rmzelle commented Jun 8, 2020

I don't think the block elements will do anything for you.

https://chemical-roles.github.io/manuscript/ suggests otherwise (note the line breaks after e.g. the title and author listings):

image

@rmzelle
Copy link
Contributor

rmzelle commented Jun 8, 2020

You could try this version as well: https://gist.github.com/rmzelle/a3f1fab95a4b136962fce5b1b7cdeaf8

@dhimmel
Copy link
Member

dhimmel commented Jun 8, 2020

Thanks so much for the help!

In manubot/manubot#110, I create references for all combinations of missing CSL JSON fields. Then I run a pandoc command to process a document that cites every reference.

I ran this for the following CSL style options:

# current manubot version
--csl=https://github.com/manubot/rootstock/raw/d61267c8ff1d7bc16644f4e15b814c1b4ca2bd31/build/assets/style.csl

# adam3smith version
--csl=https://github.com/manubot/rootstock/raw/7335415d96f5adc19fae0d03552a906f960468b9/build/assets/style.csl

# rmzelle version
--csl=https://gist.githubusercontent.com/rmzelle/a3f1fab95a4b136962fce5b1b7cdeaf8/raw/0e2478d17476c633b080b5197e145d1e2b858a2f/manubot.csl

I opened the resulting manuscript.html in my browser and printed to PDF, so I could attach to this issue. Here are the results:

The current manubot style lacks space when authors and venue is missing:

image

The style by @adam3smith adds the missing space:

image

The style by @rmzelle appears to have double new lines rather than a single newline after each group.

image

Looking through all 256 references, I don't see any reference where the @adam3smith variant is worse than the current Manubot variant! Excellent!

For reference, here is the intended style, which works when nothing is missing:

This is the title
Given-1 Family-1, Given-2 Family-2, Given-3 I. Family-3
Container Title (2019-01-01) https://manubot.org
DOI: 10.0000/fake-doi

It contains four lines: title, authors, venue + date + URL, identifiers. I think the ideal behavior would be to omit a line if everything in the line is missing. Otherwise include the line. This means that there would never be any entirely bank lines, but also content from different lines would never get put on the same line.

There are still many references, where this design is violated, For reference 15 and 16 above, the title line is combined with the date/URL. When the title is missing, the initial line is blank (rather than moving the next line to the beginning):

image

There is also a blank line when the CSL JSON contains: author, container-title, issued.

image

Now IIRC there are some CSL/pandoc-citeproc limitations that may make the desired style impossible to achieve. @adam3smith / @rmzelle do you know if that's true? With @adam3smith's missing space fix, I think references that are blatantly wrong are addressed. So it's not a huge priority for me to address the rest, unless the fix would be straightforward.

One final note: the performance of the style using pandoc-citeproc is paramount, but it would also be nice for the style to work with other processors.

</group>
<group display="block">
<text macro="author"/>
</group>
<group delimiter=" ">
Copy link
Member

Choose a reason for hiding this comment

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

The diff here makes 7335415 look larger than it actually is.

@adam3smith is the only change that everything has been wrapped in <group delimiter=" ">...</group>?

<group>
<text variable="title" font-weight="bold"/>
</group>
<group display="block">
Copy link
Member

Choose a reason for hiding this comment

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

citeproc-js isn't happy with those two blocks in there

@adam3smith I'm guessing this comment is in reference to this line and the other one below:

        <group display="block" delimiter=" · ">

By not happy, do you mean that this style fails entirely with citeproc-js? Do you think the issue is with our style or citeproc-js? I.e. is this use of block valid according the spec?

@rmzelle
Copy link
Contributor

rmzelle commented Jun 8, 2020

The style by @rmzelle appears to have double new lines rather than a single newline after each group.

The CSL specification doesn't specify what to do with empty blocks, but this style would work better if empty blocks didn't create an empty line. You could ask if pandoc-citeproc would be willing to implement that behavior.

And I didn't notice this before, but hanging-indent="true" and second-field-align="margin" should probably be mutually exclusive, so you could try deleting either attribute.

Not sure where the double lines come from.

@dhimmel
Copy link
Member

dhimmel commented Jun 9, 2020

The CSL specification doesn't specify what to do with empty blocks, but this style would work better if empty blocks didn't create an empty line. You could ask if pandoc-citeproc would be willing to implement that behavior.

@jgm does this sound like a reasonable change for pandoc-citeproc? Let me know if you want me to open an issue?

@dhimmel
Copy link
Member

dhimmel commented Jun 9, 2020

And I didn't notice this before, but hanging-indent="true" and second-field-align="margin" should probably be mutually exclusive, so you could try deleting either attribute.

Based on these docs, I think the behavior we want is second-field-align="flush":

If set, subsequent lines of bibliographic entries are aligned along the second field. With “flush”, the first field is flush with the margin.

Pandoc citeproc doesn't support this as per jgm/pandoc-citeproc#29, but the hanging-indent="true" achieves something similar (but a bit less aligned). Given this situation, I'm inclined to keep both hanging-indent and second-field-align until second-field-align is supported in Pandoc... unless this is causing the style to fail elsewhere.


Note: I realized one of Manubot's javascript plugins changes how indents are displayed on HTML versions (example):

image

The PDF versions show up the way pandoc renders them with hanging-indent (example):

image

This option is not supported by pandoc yet, but it more closely resembles
hanging-indent="true", which pandoc is currently using to layout the
bibliography.
@dhimmel
Copy link
Member

dhimmel commented Jun 9, 2020

@adam3smith / @rmzelle let me know if you have any additional comments here. Otherwise I'll go ahead and merge this PR, since while it doesn't fix everything, it fixes some stuff without breaking anything!

@AppVeyorBot
Copy link

AppVeyor build 1.0.141

Found 52 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02 for commit 5170dab is now complete....
The rendered manuscript from this build is temporarily available for download at:

@dhimmel dhimmel merged commit adc7b8b into manubot:master Jun 11, 2020
dhimmel added a commit to manubot/manubot that referenced this pull request Jun 11, 2020
merges #110

Evaluate all combinations of missing CSL JSON fields.
Example application to assess a CSL Style change at
manubot/rootstock#346 (comment)

Includes script to generate CSL Items and manuscript markdown
with all combinations of fields.
cthoyt added a commit to chemical-roles/manuscript that referenced this pull request Jun 30, 2020
* CSL: add space delimiter across the style

merges manubot/rootstock#346
closes manubot/rootstock#155
supersedes manubot/rootstock#158
tested via manubot/manubot#110
refs https://twitter.com/csl_styles/status/1270049414200074243

Add a space delimiter across the style.
Fixes some references where there is a missing space after the title,
when authors are missing.

Set second-field-align="flush"
This option is not supported by pandoc yet, but it more closely resembles
hanging-indent="true", which pandoc is currently using to layout the
bibliography.

Co-authored-by: Rintze M. Zelle <[email protected]>

* Improve themes for DOCX output

merges manubot/rootstock#348
closes manubot/rootstock#344

Update docx files that can be supplied to Pandoc via --reference-doc.

Recreate build/themes/default.docx from scratch (via a blank pandoc output).
Improves aesthetic and fixes issues like invisible table gridlines.

Add build/themes/nih-grant.docx that produces documents that adhere
to many of the National Institutes of Health (NIH) formatting requirements.
https://grants.nih.gov/grants/how-to-apply-application-guide/format-and-write/format-attachments.htm

Co-authored-by: Sebastian Karcher <[email protected]>
Co-authored-by: Rintze M. Zelle <[email protected]>
Co-authored-by: Vincent Rubinetti <[email protected]>
@cthoyt
Copy link

cthoyt commented Jun 30, 2020

@adam3smith this fixed my original issue, you can see the results in https://chemical-roles.github.io/manuscript/. Thanks so much!

ploegieku added a commit to ploegieku/2023-functional-homology-paper that referenced this pull request Aug 6, 2024
merges manubot/rootstock#346
closes manubot/rootstock#155
supersedes manubot/rootstock#158
tested via manubot/manubot#110
refs https://twitter.com/csl_styles/status/1270049414200074243

Add a space delimiter across the style.
Fixes some references where there is a missing space after the title,
when authors are missing.

Set second-field-align="flush"
This option is not supported by pandoc yet, but it more closely resembles
hanging-indent="true", which pandoc is currently using to layout the
bibliography.

Co-authored-by: Rintze M. Zelle <[email protected]>
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.

5 participants