search

citation-style-language / documentation

Citation Style Language documentation
http://citationstyles.org/
Creative Commons Attribution Share Alike 4.0 International
44 stars 21 forks source link

Clarify non-encapsulation behavior of cs:choose #116

Closed bwiernik closed 4 years ago

bwiernik commented 4 years ago

Closes https://github.com/citation-style-language/documentation/issues/65

bwiernik commented 4 years ago

@fbennett does that sound clear to you?

fbennett commented 4 years ago

It would be good for @cormacrelf or another implementer to take a look, to be sure that it's clear at that end---I'm too close to the problem to say much.

bwiernik commented 4 years ago

@jgm @inukshuk

bwiernik commented 4 years ago

@bdarcus Can we merge and revisit if someone finds this unclear? There was consensus that this is the desired behavior.

bdarcus commented 4 years ago

Consensus of whom?

Unfortunately, none of the implementers have really weighed in here.

jgm commented 4 years ago

It's clear enough to me, but a bit painful to hear -- it would have been nice to know this before I started, as it would have affected some large-scale aspects of the design that are a pain to change now!

bwiernik commented 4 years ago

Rintze, Cormac, Sylvester in their discussions on Discourse. This was just adding their conclusion to the spec.

bwiernik commented 4 years ago

@jgm What part are you finding annoying about the behavior?

inukshuk commented 4 years ago

It's been a long time, but if I remember correctly this was bringing the spec in line with citeproc-js and, particularly, existing examples in the test suite and styles which relied on the behavior. I'm sure citeproc-ruby already follows citeproc-js, too.

bwiernik commented 4 years ago

Yes, that's right.

bdarcus commented 4 years ago

Rintze, Cormac, Sylvester in their discussions on Discourse. This was just adding their conclusion to the spec.

Would help to have some links to that then. Is this one of them?

https://discourse.citationstyles.org/t/groups-variables-and-missing-dates/

And given @jgm's reaction, perhaps if we do merge, it shouldn't be to master/1.0?

bwiernik commented 4 years ago

I'll pull together some links.

To be clear, this isn't a change, but a fix to an ambiguity in the spec that ensures it aligns with the expected behavior in repository styles and with the test suite. So it should go into master/1.0--1.0 styles may depend on it.

cormacrelf commented 4 years ago

This is the relevant discussion on cs:choose. https://discourse.citationstyles.org/t/should-if-else-blocks-use-the-delimiter-from-a-parent-group/1511

Recommendations

These amendments use 'rendering elements' to refer to delimit-able things, while the Delimiter section calls them 'the output of child elements'. Calling them 'rendering elements' in addition to maintaining the category of rendering elements as including 'cs:choose' is a bit confusing. Note that the Delimiter section says this:

the delimiter attribute, whose value delimits non-empty pieces of output [...] cs:group and cs:layout (delimiting the output of the child elements)

"The output of the child elements" / "pieces of output" is actually a solid conceptual and drafting device that we can use. We're simply defining how we should "delimit" the output of two particular kinds of element. So I would go for:

Rendered output of <text macro="..."> is encapsulated: when rendering a delimiter_ on the macro call's nearest parent cs:layout, cs:group, or cs:substitute (inherited from cs:names) element, no delimiters are placed between the children output by the macro, and the macro's output is considered to be a single piece of output.

Rendered output of cs:choose (i.e. the output of the matching cs:if, cs:else-if, or cs:else) is not encapsulated: if there are multiple elements output by the chosen branch, when rendering a delimiter_ set on a the nearest parent cs:layout, cs:group, or cs:substitute [inherited from cs:names] element, they are treated as separate pieces of output and delimiters are placed between them.

It's a bit wordy, but when am I not. If you wanted to you could also remove the "encapsulated" terminology, unless (or perhaps especially if) you thought it would be useful to make other distinctions. You could also make it a lot easier to read by defining a 'delimiting element' as one of those elements (whether or not the delimiter was supplied) in the Delimiter section and referring to it twice.

Commentary

This aligns with a glossary like so:

I think that sufficiently resolves the question around "what are rendering elements?" raised in the thread(s).

There is no mention of "rendering elements" anyway. Some variables are empty, so you have to "look inside" variables to determine whether they need a delimiter. Looking inside choose is very similar. The spec clearly contemplates that the delimiters would be placed around something other than every single XML syntactic child. But it wasn't specific about how it would delimit that output, or what "pieces of of output" are. @jgm has to change his model, and funnily enough, so do I. I do not think that makes it a breaking change.

I did have to create tests e.g. this to cover it as none existed in the suite at the time, and citeproc-rs doesn't actually pass them yet. But I believe that:

Finally, if you want to tackle https://discourse.citationstyles.org/t/groups-variables-and-missing-dates/, I still stand by the 'plain'/'missing'/'important' rules I wrote down in that thread as the most clear and precise way to define it. I think we should pretty much copy it verbatim into the spec. But that's for another issue.

bwiernik commented 4 years ago

Thanks Cormac.

bdarcus commented 4 years ago

Closing in favor of #120.

bwiernik commented 4 years ago

@cormacrelf I think the ambiguities in https://discourse.citationstyles.org/t/groups-variables-and-missing-dates/ are pretty much resolved by your new pull request and https://github.com/bwiernik/documentation/commit/ebb11499082d177c9fea1825767af2879dd602db

cormacrelf commented 4 years ago

I'll continue over at #106