citation-style-language / test-suite

11 stars 13 forks source link

page_Expand test sometimes updates page range delimiter, sometimes not #54

Closed Jason-Abbott closed 1 year ago

Jason-Abbott commented 1 year ago

The expected output for page_Expand sometimes retains the original page range delimiter character (retaining a dash) and sometimes it replaces it with the locale standard (converting dash to en-dash).

So always keeping the existing delimiter doesn’t work and always replacing it doesn’t work. I can’t discern the expected logic.

It seems like the range character should always be updated to the standard when re-formatting a range. What am I missing?

Example: minimal, at 110–115
Example: prefix on first number only, at N110-5
Example: same prefix on both numbers, at N110–N115
Example: prefix on last number only, at 110-N6
Example: different prefixes on both numbers, at N110-P5
Example: leading number before prefix, at 123N110-N5
Example: multiple ranges, at 123N110-N5, 456K200-99
Example: first less than second, at 123N110-N5, 000c23-22
Jason-Abbott commented 1 year ago

This seems to be pertinent documentation:

The “locator” variable is always rendered with an en-dash replacing any hyphens. For the “page” variable, this replacement is only performed if the page-range-format attribute is set on cs:style

In this case, the page-range-format attribute is set so it seems, contrary to the test, that all dashes should be en-dashes.

adam3smith commented 1 year ago

I'm pretty sure the rationale here is that not all hyphens indicate ranges -- they can also, e.g., be part of an article number, in which case flipping them is clearly wrong. So the test assumes that if you have just numbers or you have numbers with the same prefix, that's a range, otherwise it's not. The relevant commit changing the test (which did use to be all en-dashes) also suggests that rationale.

Jason-Abbott commented 1 year ago

Okay, that makes sense. I’ll close this issue. Perhaps confusing is that some of the unchanged dashes are called “ranges” in the test. I guess that’s just how they began, prior to the commit you linked.

Jason-Abbott commented 1 year ago

P.S. I’ll be sure to check commit history on tests before opening issues from now on. 👍

adam3smith commented 1 year ago

I think we'd also take PR's with comments on tests you find confusing. Ideally it shouldn't need a 7 year-old memory of a discussion on the Zotero forums to figure out the rationale for a test ;)

bdarcus commented 1 year ago

Yes, see #51