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

Don't treat <choose> as a grouping/rendering element #83

Closed
bwiernik opened this issue Aug 29, 2024 · 9 comments
Closed

Don't treat <choose> as a grouping/rendering element #83

bwiernik opened this issue Aug 29, 2024 · 9 comments

Comments

@bwiernik
Copy link

Noticed this behavior here: citation-style-language/styles#6500

For this structure:

<macro name="publisher">
    <group delimiter=", ">
      <choose>
        <if type="article-journal article-magazine" match="none">
          <text variable="genre"/>
          <text variable="publisher"/>
          <text variable="publisher-place"/>
        </if>
      </choose>
    </group>
  </macro>

The expected output is: Genre, Publisher, Place

The group attributes like delimiter should pass through <choose> and apply to the individual rendering elements inside, with the same behavior as if the elements were supplied directly without <choose>. <choose> should not create an implicit <group>.

I find that the current behavior not infrequently produces surprising output for the checks we run on the CSL style repository. Would it be possible to update citeproc-ruby's behavior here?

Thanks!

@inukshuk
Copy link
Owner

inukshuk commented Sep 1, 2024

Thanks @bwiernik -- it's been awhile since I've looked at this so my apologies while I get up to speed! Is the issue here that the three text nodes are rendered without the delimiter? Specifically, this note in the spec isn't being followed when rendering the text nodes?

@bwiernik
Copy link
Author

bwiernik commented Sep 1, 2024

Yes the issue is that the 3 nodes are being rendered without the delimiter

inukshuk added a commit that referenced this issue Sep 1, 2024
@inukshuk
Copy link
Owner

inukshuk commented Sep 1, 2024

Yes you're right, I added your example as a test case and it's definitely rendering without the delimiter at the moment.

@inukshuk
Copy link
Owner

inukshuk commented Sep 1, 2024

OK your example should be easy to fix, however, more generally, are ancestor delimiters computed based on the style structure only or based on the current rendering tree? For example if I omit the <group> in your example so that the macro contains only the <choose> element and if I then render that macro inside a group, then would you expect to find the delimiter attribute on that node?

@inukshuk
Copy link
Owner

inukshuk commented Sep 1, 2024

I guess, using the rendering tree (i.e., inherit delimiters from outside marcros) is potentially dangerous, because this way you'd always reach at least the <layout> and therefore layouts with delimiters couldn't easily have choose blocks without a delimiter (they'd basically have to add extra <group> elements without delimiters). So I guess the intention is for delimiters to be inherited only through the style nodes?

@inukshuk
Copy link
Owner

inukshuk commented Sep 1, 2024

OK I implemented it this way for now. Delimiters can't be inherited from outside a macro, so basically, the when a choose node produces output from more than one rendering element, they are joined using the delimiter of the closest group or layout element above it. If that's the way it's supposed to be we can close this and I'll push a new version of the Gem.

@inukshuk inukshuk reopened this Sep 1, 2024
@bwiernik
Copy link
Author

bwiernik commented Sep 2, 2024

That's correct, there is no need to propagate delimiters into a macro call.

@bwiernik
Copy link
Author

bwiernik commented Sep 2, 2024

Thanks for looking into this so quickly!

@inukshuk
Copy link
Owner

inukshuk commented Sep 2, 2024

Great, thanks!

I pushed version 2.1.0 to RubyGems.

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

No branches or pull requests

2 participants