Ihor Radchenko <[email protected]> writes:
> Christian Moe <[email protected]> writes:
> See my comments inline.
> I also attached all my comments in a form of suggested changes on top of
> your patch.

Thanks! Comments and some additional suggestions inline; let me know
what you think, and I can make another pass.

>> +- When the =csl= processor is used, "locators" such as page references
>> +  will be recognized and rendered according to the selected CSL style
>> +  (other processors will render them verbatim).  A locator is a part
>> +  of the citation suffix that starts with a locator term and ends with
>> +  the last comma or digit in the suffix, whichever comes last. Locator
>> +  terms include "p.", "page", and "chap."; more terms are listed
>> +  below.  In the following example, the locator =p. 3-4= might be
>> +  rendered "pp. 3--4" or just "3--4" in export, depending on the CSL
>> +  style.
>> +
>> +  : [cite:see @Tarski1965 p. 3-4 for an example]
>
> I think that this list is sprawling a bit too much now. Extended
> clarification may better be moved down, into separate paragraphs.

Agreed.

> I also reviewed which variants are supported by which processors and
> incorporated. I think you missed "note" style from oc-basic (I did not
> yet put it).

Yes, you're right, so we should add `=note= (=ft=)' for the basic
processor only. Probably at the end of the list. And possibly with a
parenthetic or footnote explanation that CSL and Bib(La)TeX processors
use separate footnote style files rather than in-citation styles.   

(I made the mistake of thinking that the styles for the csl processor
was the comprehensive list of available styles, and tested the
processors against it, failing to check the code commentary for
oc-basic.el. Conversely, this reminded me to include =nocite= for
natbib; the oc-natbib.el commentary omits it.)

Having spent a little more time with the basic processor, I think the
manual should also explicitly include the disclaimer from oc-basic.el:
"this citation processor is meant to be a proof of concept, and possibly
a fall-back mechanism when nothing else is available. It is too limited
for any serious use case." Perhaps shortened to: "The =basic= citation
processor is not meant for serious use, but only as a proof of concept
and a fall-back mechanism." Without this disclaimer, we set people up for
disappointment if they try citations right out of the box.

>>  The only mandatory elements are:
>>  
>>  - The =cite= keyword and the colon.
>>  - The =@= character immediately preceding each key.
>>  - The brackets surrounding the citation(s) (group).
>
> This feels a bit out of scope after elaborate discussion about locators
> and styles. I moved it up.

Yes, that's probably better.

>> +#+cindex: styles, for citations
>> +Citation styles and variants can be specified by their full name
>> +(e.g., =author/bare=) or shortcut (e.g., =a/b=).  The following
>> +citation styles are supported by the =csl= processor:
>> +
>> +- default style :: Provide a typical author-date citation in
>> +  parentheses.  Variants: =bare=, =caps=, and =bare-caps=.
>> +  
>> +- =author= (=a=) :: As default, but suppress the year.  Variants:
>> +  =bare=, =caps=, =full=, and all their combinations.
>> ...
>
> Compared to the earlier list with examples, this reads much less
> clearly.

I'm not sure what you're comparing with here. The list itself was pretty
much unchanged from my first patch, and it will be the first such list
to appear in the manual. Perhaps you're referring to the additional info
I added below it.

> I moved the examples right into the list items here. Note that
> it revealed that some examples are missing.

Yes. I was trying to keep things simple: 1) to avoid complex layout with
tables inside a list and 2) to provide illustrative examples of
citations, rather than illustrating all possible examples. Hence a list
of styles and variants, followed by a table of example citations
(intentionally incomplete), followed by a list of what processors
support what backend. I first experimented with something more similar
to what you've done, but I thought it made the style-list items
sprawling and that this approach was actually clearer.

That's my rationale, but your way also makes sense, and the complex
layout actually looks pretty readable. It compels us to supply examples
for all items for consistency, though, and that leads to duplication,
e.g.:

> +- default style =nil= (<omitted>) :: Provide a typical author-date
> +  citation in parentheses.\\
> [...]
> +
> +  | =[cite:@dewaal06 p. 5]=            | (de Waal et al. 2006, 5) |
> +  | =[cite//bare-caps:@dewaal06 p. 5]= | De Waal et al. 2006, 5   |
> +  | =[cite//bc:@dewaal06 p. 5]=        | De Waal et al. 2006, 5   |
> [...]
>  - =bare-caps= (=bc=) :: Combine =bare= and =caps=.
> +
> +  | =[cite//bare-caps:@dewaal06 p. 5]= | De Waal et al. 2006, 5   |
> +  | =[cite//bc:@dewaal06 p. 5]=        | De Waal et al. 2006, 5   |
>    
But I can try to deduplicate a bit and add the missing examples, if it's
your call that we should do it this way.

>> +#+cindex: locators
>> +CSL locator terms include
>> +- "book", "bk.", "bks." 
>> +- "chapter", "chap.", "chaps." 
>> +- "column", "col.", "cols."
>> ...
>
> This is (1) too far from where we discuss locators; (2) feels more like
> a footnote. I made it such.

Agreed. As an afterthought, we can probably change "include" to "are"; I
think this is the comprehensive list.

Yours,
Christian

Reply via email to