search

inukshuk / citeproc-ruby

A Citation Style Language (CSL) Cite Processor
101 stars 22 forks source link

Can't get date ranges to render #58

Open jrochkind opened 6 years ago

jrochkind commented 6 years ago

I am having trouble getting date ranges to render.

I am not sure if I am doing something wrong, it is a bug in my input, or it is a bug in the CSL style (or perhaps the CSL style I am using refuses to do it?). But looking at the CSL docs and playing with custom CSL styles... I don't think it's the CSL style.

I don't see any examples of date ranges in spec/citeproc/ruby/renderer/date_spec.rb. Or can't find any other examples in specs to use as a guide.

Is it possible date ranges don't work? Or aren't being properly imported from CSL data input json?

Can you provide any advice, or a working example of something rendering a date range, like "1900-1910" when that's in the citation?

inukshuk commented 6 years ago

Here is a test case that currently fails and judging by this TODO comment it is indeed missing. Sorry! I'll try to take a stab at it over the weekend.

jrochkind commented 6 years ago

Oh, okay, glad to have an explanation!

I guess travis isn't running the cucumber features, only the rspec? Is why the test case can fail with travis showing green?

jrochkind commented 6 years ago

Also I don't neccesarily expect you to fix this right away, although I'd of course be quite happy if you did!

Alternatives are listing it in the README to note that it doesn't work, and giving me some advice/guidance in where I find a spec (or automated tests from a non-ruby project?) for how it's supposed to behave and I'll see if I can get to it.

inukshuk commented 6 years ago

This is the relevant section in the spec; in the folder of the test case I linked to above there are many other tests (just grep for 'range' in that folder, e.g. this one) -- they are in the citeproc repo because I used to run them against citeproc-ruby and -js (via Ruby); one of the main reasons for the repo split. These tests are from citeproc-js and many of them test specific citeproc-js features that's why they are not run on Travis CI.

jrochkind commented 6 years ago

Hey @inukshuk , I suspect you didn't have time to get to this this weekend -- I'll try to get a PR this week, or else next. Let me know if you're already working on it though, so I don't duplicate what you are already working on! Otherwise I'll try to get it in!

jrochkind commented 6 years ago

Woah, getting date ranges to work within the current architecture is harder than I expected, heh. Still working on it, but any advice for how you might go about it appreciated.

inukshuk commented 6 years ago

Yeah, I got stuck over the weekend, because I fell into a rabbit hole (this usually happens with CSL): I'm not sure how season ranges are supposed to be rendered (since the season is set for the whole date not the part), but I might be missing something.

I pushed #59 which is as far as I got. I added three line comments -- most notably, something along the lines of date.delimit_range_at should be added to CiteProc::Date, returning the name of the first part (day, month, year, in that order, not the order of the date-part notes) that is different. This also doesn't address 'open' ranges yet (which the spec does not mention I believe?).

inukshuk commented 6 years ago

OK, cleared up the season issue. Seems like citeproc-js interprets month-part 13-16 as seasons. In addition, it would make sense to accept 21-24 there as well (the numbers used by ISO 8601 for seasons, independent of hemisphere).