Make publisher a name variable. #368

bwiernik · 2020-08-18T18:47:37Z

Description

Make publisher a name variable and add place attribute to the name variable schema. This will permit correct rendering of multiple publisher/publisher places.

Also:

adds new cs:name @form "none" to omit name and et-al elements from a names citation (e.g., to permit only the publisher-place to be rendered)
adds @form attribute to cs:et-al. form="none" suppresses the et-al term. The options here are consistent with the same attribute used in cs:citation-label-format

Existing styles can be programmatically updated to achieve the current output behavior by replacing:

<text variable="publisher"/>

with

<names variable="publisher">
  <name form="long" delimiter="; "/>
</names>

and

<text variable="publisher-place"/>

with

<names variable="publisher-place">
  <name form="none"/>
  <place delimiter="; "/>
</names>

Fixes #13

Type of change

Please delete options that are not relevant.

Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

bdarcus

Left some feedback, but was hard to tell the diff since you moved names to a separate file.

Any chance you have a diff you can post of just the changes there?

schemas/input/csl-data.json

schemas/styles/csl-names.rnc

bwiernik · 2020-08-18T19:31:21Z

Any chance you have a diff you can post of just the changes there?

Perhaps look at the individual commits? I initially moved the names to a new file without any changes.

bdarcus · 2020-08-18T19:32:45Z

That works; I missed that.

bdarcus · 2020-08-18T19:36:10Z

Actually, no it doesn't. The first commit is that; there's no commit with a diff of the main schema file.

Or if it's straightforward, I suppose you could just tell us what changed on that schema.

bwiernik · 2020-08-18T19:44:59Z

On the main schema file, I just removed the cs:names content and the names-related style options.

Co-authored-by: Bruce D'Arcus <[email protected]>

denismaier · 2020-08-18T21:56:57Z

Make publisher a name variable and add place attribute to the name variable schema. This will permit correct rendering of multiple publisher/publisher places.

In addition, this PR adds a new option for @form ("none"), right? Anything else? Maybe you could add that to the initial comment.

bwiernik · 2020-08-18T22:04:58Z

Make publisher a name variable and add place attribute to the name variable schema. This will permit correct rendering of multiple publisher/publisher places.

In addition, this PR adds a new option for @form ("none"), right? Anything else? Maybe you could add that to the initial comment.

Update.

Also:

adds new cs:name @Form "none" to omit name and et-al elements from a names citation (e.g., to permit only the publisher-place to be rendered)

adds @Form attribute to cs:et-al. form="none" suppresses the et-al term. The options here are consistent with the same attribute used in cs:citation-label-format

denismaier · 2020-08-19T08:58:32Z

Maybe I've missed it, but I've only seen delimiters between names and between places. But how do you actually specify the colon between the place and the publisher? Simply an affix on the name element?

schemas/styles/csl-names.rnc

bwiernik · 2020-08-19T09:53:07Z

@denismaier Yes, like with a label.

denismaier · 2020-08-19T10:09:32Z

@denismaier Yes, like with a label.

I've tested with the 1.1 branch:

<names variable="author">
  <label suffix=". "/>
</names>

Jing complains about the suffix on label. "Attribute 'suffix' not allowed here."

But that is:

<names variable="author">
 <name suffix=". "/>
 <label/>
</names>

So the idea is to do this?

<names variable="publisher">
 <place/>
 <name prefix=": "/>
</names>

denismaier · 2020-08-19T10:14:22Z

If I remember correctly, 1.0.1 had affixes on cs:label. Why have we removed them?

bwiernik · 2020-08-19T18:39:45Z

That's a bug. label.attributes-shared should be part of the names.label list.

bwiernik · 2020-08-19T21:56:49Z

@denismaier That's fixed now.

denismaier · 2020-08-19T22:01:07Z

But it is still only on label, not on place, right? Is that on purpose?

bwiernik · 2020-08-19T22:04:04Z

Nope! Fixed.

bwiernik · 2020-08-19T22:05:11Z

I added most of the text formatting attributes. Any that place shouldn't have? Only one I'm not really sure about is strip-periods?

      affixes,
      font-formatting,
      strip-periods,
      text-case,

denismaier · 2020-08-19T22:14:49Z

Not sure.
On the other hand, do we need an attribute to specify state formatting rules? "Cambridge/MA" vs "Cambridge (MA)"

bwiernik · 2020-08-19T22:32:48Z

No, we've never had something like that before where we edit the contents of a string. I suggest we not do that now.

bwiernik · 2020-09-05T15:34:56Z

@bdarcus RTM?

Versus separate names.abbreviation, names.alternate, or names.place elements

bwiernik · 2020-11-02T17:09:56Z

@bdarcus @denismaier I reworked the names structure to use a single cs:names element for all of place, abbreviation, and alternate. Can you look over the csl-names.rnc schema again?

denismaier · 2020-11-03T15:18:36Z

@bwiernik Can you please give a brief summary of your changes? And perhaps a couple of examples. It's been a while since we've discussed this.

bwiernik · 2020-11-03T20:38:18Z

The current state: If a style wants to show an institution abbreviation, an alternative name (e.g., a pseudonym or screenname), or a place, they would add a cs:component element as a child to cs:names.

This is a change from the previous version, where each of these three had separate child elements on cs:names.

So, for example, to render publisher and publisher place:

<names variable="publisher">
  <component component="place" suffix=": "/>
  <name form="long"/>
</names>

So, to render an institutional name in long form with the abbreviation the first time, then just the abbreviation subsequently:

<names variable="author">
  <name>
    <name-type name="personal" form="short"/>
    <name-type name="institutional" form="long" form-subsequent="short"/>
  <name/>
   <component component="abbreviation" prefix=" [" suffix="]"/>
</names>

To render a name with its alternative form:

<names variable="author">
  <name form="long" name-as-sort-order="all"/>
  <component component="alternative" prefix=" [" suffix="]"/>
</names>

A single cs:names element might have multiple of these, such as both abbreviation and alternative.

bdarcus · 2020-11-03T20:47:58Z

In general and at first glance, looks good ... except for that component/@component duplication. Is that correct, or a typo?

bwiernik · 2020-11-03T21:25:41Z

In general and at first glance, looks good ... except for that
component/@component duplication.

Is that correct, or a typo?

Not following. The name of the element is component, and it has a component attribute indicating which component to render.

bdarcus · 2020-11-03T21:46:31Z

A component of a component, same node name for element and attribute, makes no sense. Maybe we need a different name for that attribute? I'll take a closer look later to see if I have a concrete suggestion.

bwiernik · 2020-11-03T22:30:46Z

I really don't see the problem. The element is "Add another component". The attribute is "which component should we add?". It's not ambiguous. We could rename the attribute which or which-component, but why make it more ambiguous or verbose? The closest analogue we have currently is the cs:name-type element has the name attribute to mean "Why type of name is it?" Honestly, that would be clearer as attribute name-type to me, too. But I don't have a strong preference.

Edit: Oh, name there refers to the name of the element, not a name variable.

denismaier · 2020-11-05T15:42:47Z

Ok, this change does not affect the general behaviour here, or does it? It's only that there's now one element instead of multiple, correct?

Anyway, it's hard for me to get a grasp of this using the web interface. Is it possible to checkout the PR or a certain commit? I could do some tests then in with Atom or Oxygen. @bdarcus ?

bwiernik · 2020-11-05T17:08:07Z

Exactly.

See here for how to check out a PR http://stackoverflow.com/questions/27567846/ddg#30584951

bdarcus · 2021-03-06T22:34:06Z

Where do we stand with this one?

The issue does flag an unaddressed requested change, which I assume @bwiernik did address. If yes, can you please click that you did so, so that goes away?

And on a related note, once PRs are ready, nothing has changed; right? Since this has no conflicts, I can just do the merge?

I will plan to follow @denismaier's thought to actually test it soon.

bwiernik · 2021-03-27T22:27:38Z

@bdarcus Is this good to merge now?

bdarcus · 2021-03-27T22:42:22Z

@bdarcus Is this good to merge now?

Can you just respond to my last note, then I can test it tomorrow.

IIRC, the only issues was the name/name node name issue.

bwiernik · 2021-03-27T22:45:53Z

I resolved the component element/attribute issue yes.

resolved

bdarcus

I added a few minor requests. In the meantime, I'll spend some time testing the rnc schema changes.

When you resolve, can please indicate that you've done so by clicking whatever github button. I forget how to actually do so, but I think it's straightforward.

EDIT: I just tried to load the csl.rnc file in nxml-mode, and got the following error:

nxml-display-file-parse-error: Reference to undefined pattern names-inheritable-options

Is it because you don't include the csl-names.rnc file?

If I do that, the rnc-validate-format.sh script reports ...

.../csl-names.rnc:309:7: error: syntax error

... though I'm confused by that.

bdarcus · 2021-03-28T13:54:36Z

schemas/input/csl-data.json

@@ -243,7 +243,18 @@
            "alternate": {
              "title": "Alternative name, such as screen name for online item or real name of pseudonymous author.",
              "description": "E.g. rendered as 'Smith, J. [@JSmith]'",


Can you please include "examples" in the json schema, as elsewhere, @bwiernik?

bdarcus · 2021-03-28T13:55:35Z

schemas/input/csl-data.json

@@ -472,6 +483,14 @@
            "$ref": "#/definitions/name-variable"
          }
        },
+        "publisher": {
+          "title": "Publisher of the item",
+          "description": "e.g., [{'institution': 'Publisher', 'place': 'London, UK'}], [{'institution': 'Publisher A', 'place': 'New York, NY, US'}, {'institution': 'Publisher B', 'place': 'Berlin, Germany'}]",


Same here; the "e.g." stuff should go in "examples"; see the title properties, or "custom".

bdarcus · 2021-03-28T13:56:52Z

schemas/input/csl-data.json

@@ -634,7 +653,7 @@
          "type": "string"
        },
        "original-publisher-place": {
-          "description": "[Deprecated - Use `original` related `publisher-place` property instead]",
+          "description": "[Deprecated - Use `original` related `publisher` property `place` property instead]",


Suggested change

"description": "[Deprecated - Use `original` related `publisher` property `place` property instead]",

"description": "[Deprecated - Use `original` related `publisher` property `place` instead]",

Right?

bdarcus · 2021-03-28T14:17:36Z

schemas/styles/csl-names.rnc

+
+      ## If name="alternate", use to apply separate formatting to individual name parts or to personal vs. institutional names 
+      name.name-part,
+      name.name-type


Suggested change

name.name-type

name.name-type }

I think this is the change you need for the one error, but then I get others.

Unless I'm doing something wrong with git, maybe double check the syntax problems and let me know when I can try again?

EDIT: obviously the CI isn't working correctly, as it should have flagged these issues.

bdarcus · 2021-10-16T16:11:47Z

Status of this PR?

Also, now that you're working on 1.1, any objections @cormacrelf?

bwiernik added 6 commits August 18, 2020 13:33

Moves names schema to separate file

b5b349c

Add publisher style code to names style schema

af15950

Rename form="no-name" to "none" for consistency.

7564183

Move publisher and publisher-place to names

a5e61cf

Make "alternate" element a name array instead of a string.

849d238

Fix wording

cc28165

bwiernik requested review from bdarcus and denismaier August 18, 2020 18:52

bdarcus previously requested changes Aug 18, 2020

View reviewed changes

schemas/input/csl-data.json Show resolved Hide resolved

schemas/input/csl-data.json Show resolved Hide resolved

schemas/input/csl-data.json Outdated Show resolved Hide resolved

schemas/styles/csl-names.rnc Outdated Show resolved Hide resolved

bwiernik and others added 3 commits August 18, 2020 16:24

annotate place in csl-data

30fd938

Add publisher data example

7f09aa1

Fix wording

569b3d9

Co-authored-by: Bruce D'Arcus <[email protected]>

denismaier requested changes Aug 19, 2020

View reviewed changes

schemas/styles/csl-names.rnc Outdated Show resolved Hide resolved

schemas/styles/csl-names.rnc Outdated Show resolved Hide resolved

Restore label.attributes-shared

7359534

Add formatting attributes to cs:place

9a8fd01

Switch to unified names.component

c798259

Versus separate names.abbreviation, names.alternate, or names.place elements

bwiernik requested review from denismaier and bdarcus November 2, 2020 17:06

Rename component attribute to name

4e0f8f6

All multiple cs:component elements

d3692a8

bdarcus requested changes Mar 28, 2021

View reviewed changes

bdarcus reviewed Mar 28, 2021

View reviewed changes

	"description": "[Deprecated - Use `original` related `publisher` property `place` property instead]",
	"description": "[Deprecated - Use `original` related `publisher` property `place` instead]",

Make publisher a name variable. #368

Are you sure you want to change the base?

Make publisher a name variable. #368

Conversation

bwiernik commented Aug 18, 2020 • edited Loading

Description

Type of change

bdarcus left a comment

Choose a reason for hiding this comment

bwiernik commented Aug 18, 2020

bdarcus commented Aug 18, 2020 via email

bdarcus commented Aug 18, 2020

bwiernik commented Aug 18, 2020

denismaier commented Aug 18, 2020

bwiernik commented Aug 18, 2020

denismaier commented Aug 19, 2020

bwiernik commented Aug 19, 2020

denismaier commented Aug 19, 2020 • edited Loading

denismaier commented Aug 19, 2020

bwiernik commented Aug 19, 2020

bwiernik commented Aug 19, 2020

denismaier commented Aug 19, 2020

bwiernik commented Aug 19, 2020

bwiernik commented Aug 19, 2020 • edited Loading

denismaier commented Aug 19, 2020 • edited Loading

bwiernik commented Aug 19, 2020

bwiernik commented Sep 5, 2020

bwiernik commented Nov 2, 2020

denismaier commented Nov 3, 2020 • edited Loading

bwiernik commented Nov 3, 2020

bdarcus commented Nov 3, 2020 via email • edited Loading

bwiernik commented Nov 3, 2020

bdarcus commented Nov 3, 2020 via email • edited Loading

bwiernik commented Nov 3, 2020 • edited Loading

denismaier commented Nov 5, 2020

bwiernik commented Nov 5, 2020

bdarcus commented Mar 6, 2021 • edited Loading

bwiernik commented Mar 27, 2021

bdarcus commented Mar 27, 2021

bwiernik commented Mar 27, 2021

bdarcus left a comment • edited Loading

Choose a reason for hiding this comment

bdarcus Mar 28, 2021

Choose a reason for hiding this comment

bdarcus Mar 28, 2021

Choose a reason for hiding this comment

bdarcus Mar 28, 2021

Choose a reason for hiding this comment

bdarcus Mar 28, 2021 • edited Loading

Choose a reason for hiding this comment

bdarcus commented Oct 16, 2021 • edited Loading

bwiernik commented Aug 18, 2020 •

edited

Loading

denismaier commented Aug 19, 2020 •

edited

Loading

bwiernik commented Aug 19, 2020 •

edited

Loading

denismaier commented Aug 19, 2020 •

edited

Loading

denismaier commented Nov 3, 2020 •

edited

Loading

bdarcus commented Nov 3, 2020 via email •

edited

Loading

bdarcus commented Nov 3, 2020 via email •

edited

Loading

bwiernik commented Nov 3, 2020 •

edited

Loading

bdarcus commented Mar 6, 2021 •

edited

Loading

bdarcus left a comment •

edited

Loading

bdarcus Mar 28, 2021 •

edited

Loading

bdarcus commented Oct 16, 2021 •

edited

Loading